Re: [Qemu-devel] [PATCH V3 WIP 3/3] disable vhost_verify_ring_mappings check

[Nicholas A. Bellinger]

On Thu, 2013-03-28 at 11:03 +0100, Paolo Bonzini wrote:
> Il 28/03/2013 10:04, Michael S. Tsirkin ha scritto:
> >>> > > Got ranges_overlap for vq: 0 ring_phys: 0 ring_size: 1028
> >>> > > Checking vq: 1 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>.
> >>> > > Got ranges_overlap for vq: 1 ring_phys: 0 ring_size: 1028
> >>> > > Checking vq: 2 ring_phys: ed000 ring_size: 5124 >>>>>>>>>>>>>>>>>>.
> >>> > > Calling l: 5124 for start_addr: c0000 for vq 2
> >>> > > Unable to map ring buffer for ring 2
> >>> > > l: 4096 ring_size: 5124
> > okay so the ring address is within ROM.
> > Unsurprisingly it fails.
> > bios should stop device before write protect.
> > 
> 
> The above log is very early, when everything is RAM:
> 
>   vhost_set_memory: section: 0x7fe2801f2b60 section->size: 2146697216 add: 0
>   Before vhost_verify_ring_mappings: start_addr: c0000 size: 2146697216
> 
> The rings are not within ROM.  ROM is at 0xc0000-0xcc000 according to the
> PAM registers.
> 
> The way I followed the debug output, "Got ranges_overlap" means 
> actually "bailing out because ranges do not overlap".

Yes, this is when !ranges_overlap() is hit in
vhost_verify_ring_mappings(), so the offending cpu_physical_memory_map()
is skipped..

> In particular, 
> here all three virtqueues fail the test, because this is the ROM area 
> 0xc0000..0xc7fff:
> 
>   vhost_set_memory: section: 0x7fe2801f2aa0 section->size: 32768 add: 1
>   Before vhost_verify_ring_mappings: start_addr: c0000 size: 32768
>   Checking vq: 0 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>.
>   Got ranges_overlap for vq: 0 ring_phys: 0 ring_size: 1028
>   Checking vq: 1 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>.
>   Got ranges_overlap for vq: 1 ring_phys: 0 ring_size: 1028
>   Checking vq: 2 ring_phys: ed000 ring_size: 5124 >>>>>>>>>>>>>>>>>>.
>   Got ranges_overlap for vq: 2 ring_phys: ed000 ring_size: 5124
> 
> Just below, vhost looks at the large RAM area starting at 0xc8000
> (it's large because 0xf0000..0xfffff is still RAM):
> 
>   vhost_set_memory: section: 0x7fe2801f2aa0 section->size: 2146664448 add: 1
>   Before vhost_verify_ring_mappings: start_addr: c8000 size: 2146664448
>   Checking vq: 0 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>.
>   Got ranges_overlap for vq: 0 ring_phys: 0 ring_size: 1028
>   Checking vq: 1 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>.
>   Got ranges_overlap for vq: 1 ring_phys: 0 ring_size: 1028
>   Checking vq: 2 ring_phys: ed000 ring_size: 5124 >>>>>>>>>>>>>>>>>>.
>   Calling l: 5124 for start_addr: c8000 for vq 2
> 
> Here vq 0 and 1 fail the test because they are in low RAM, vq 2 passes.
> 
> After 0xf0000..0xfffff is marked readonly,

Btw, the first vhost_set_memory() and failing
vhost_verify_ring_mappings() do not occur until the
pci_config_writeb(..., 0x31) code is executed in
src/shadow.c:make_bios_readonly_intel() below:

static void
make_bios_readonly_intel(u16 bdf, u32 pam0)
{
    // Flush any pending writes before locking memory.
    wbinvd();

    // Write protect roms from 0xc0000-0xf0000
    u32 romend = rom_get_last(), romtop = rom_get_max();
    int i;
    for (i=0; i<6; i++) {
        u32 mem = BUILD_ROM_START + i * 32*1024;
        u32 pam = pam0 + 1 + i;
        if (romend <= mem + 16*1024 || romtop <= mem + 32*1024) {
            if (romend > mem && romtop > mem + 16*1024)
                pci_config_writeb(bdf, pam, 0x31);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

            break;
        }
        pci_config_writeb(bdf, pam, 0x11);
    }

    // Write protect 0xf0000-0x100000
    pci_config_writeb(bdf, pam0, 0x10);
}

Up until this point, vhost_verify_ring_mappings() is not called by
vhost_set_memory() as vhost_dev_start() has not been invoked to set
vdev->started yet..

>  vhost looks at the RAM
> between 0xc9000 and 0xf0000:
> 
>   vhost_set_memory: section: 0x7fe2801f2aa0 section->size: 159744 add: 1
>   Before vhost_verify_ring_mappings: start_addr: c9000 size: 159744
>   Checking vq: 0 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>.
>   Got ranges_overlap for vq: 0 ring_phys: 0 ring_size: 1028
>   Checking vq: 1 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>.
>   Got ranges_overlap for vq: 1 ring_phys: 0 ring_size: 1028
>   Checking vq: 2 ring_phys: ed000 ring_size: 5124 >>>>>>>>>>>>>>>>>>.
>   Calling l: 5124 for start_addr: c9000 for vq 2
> 
> and the ROM between 0xf0000 and 0xfffff, which no ring overlaps with:
> 
>   vhost_set_memory: section: 0x7fe2801f2aa0 section->size: 65536 add: 1
>   Before vhost_verify_ring_mappings: start_addr: f0000 size: 65536
>   Checking vq: 0 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>.
>   Got ranges_overlap for vq: 0 ring_phys: 0 ring_size: 1028
>   Checking vq: 1 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>.
>   Got ranges_overlap for vq: 1 ring_phys: 0 ring_size: 1028
>   Checking vq: 2 ring_phys: ed000 ring_size: 5124 >>>>>>>>>>>>>>>>>>.
>   Got ranges_overlap for vq: 2 ring_phys: ed000 ring_size: 5124
> 
> 
> 
> SeaBIOS is indeed not initializing vqs 0/1 (the control and event 
> queues), so their ring_phys is 0.  But the one that is failing is vq 2, 
> the first request queue.
> 
> Your patch seems good, but shouldn't fix this problem.
> 
> Paolo
> --
> To unsubscribe from this list: send the line "unsubscribe target-devel" in
> the body of a message to address@hidden
> More majordomo info at  http://vger.kernel.org/majordomo-info.html