[SeaBIOS] [PATCH] pci: don't map virtio 1.0 storage devices above 4G

[Marcel Apfelbaum]

On 09/12/2016 06:46 AM, Michael S. Tsirkin wrote:
> On Sun, Sep 11, 2016 at 08:23:34PM +0300, Marcel Apfelbaum wrote:
>> Otherwise SeaBIOS can't access virtio's modern BAR.
>>
>> Signed-off-by: Marcel Apfelbaum <marcel at redhat.com>
>> ---
>>
>> Hi,
>>
>> If there is no room to map all MMIO BARs into the 32-bit PCI window,
>> SeaBIOS will re-allocate all 64-bit MMIO BARs into over-4G space.
>>
>> Virtio 1.0 block devices (virtio-blk/virtio-scsi) use a 64-bit BAR
>> unusable by SeaBIOS if mapped over 4G space, preventing the system
>> to boot.
>>
>> The simplest solution is to follow the xhci model and simply
>> skip migrating the virtio 1.0 modern bar into over-4G space.
>>
>>
>> In order to reproduce the problem use:
>>    -device virtio-blk-pci, ...\
>>    -object memory-backend-file,id=mem,size=4G,mem-path=/dev/hugepages,share=on \
>>    -device ivshmem-plain,memdev=mem \
>>
>>
>> Thanks,
>> Marcel
>
>>  src/fw/pciinit.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
>> index 35d9902..3b76e66 100644
>> --- a/src/fw/pciinit.c
>> +++ b/src/fw/pciinit.c
>> @@ -655,6 +655,13 @@ static void pci_region_migrate_64bit_entries(struct pci_region *from,
>>              continue;
>>          if (entry->dev->class == PCI_CLASS_SERIAL_USB)
>>              continue;
>> +        if (entry->dev->vendor == PCI_VENDOR_ID_REDHAT_QUMRANET &&
>> +            (entry->dev->device == PCI_DEVICE_ID_VIRTIO_BLK_09  ||
>> +             entry->dev->device == PCI_DEVICE_ID_VIRTIO_BLK_10  ||
>> +             entry->dev->device == PCI_DEVICE_ID_VIRTIO_SCSI_09 ||
>> +             entry->dev->device == PCI_DEVICE_ID_VIRTIO_SCSI_10))
>> +            continue;
>> +
>>          // Move from source list to destination list.
>>          hlist_del(&entry->node);
>>          hlist_add(&entry->node, last);
>> --
>> 2.5.5
>
> What if guest is booting from the network?
>

No harm done. And the Guest OS can re-map the hardware registers to 64-bit if desired.

> Rather than special-case virtio storage,

It is not a special case. The system can boot from usb, storage or network. (I might forget something here :))
Each hardware vendor needs to ensure their hardware can be used
by the firmware and this is what we are doing here.

For example commit (a247e678) pci: don't map usb host adapters above 4G
does that for all USB controllers.

  it seems to me that the right
> thing to do is to only allocate resources for the boot devices, and rely
> on the OS to allocate resources for the rest.  IIUC this is typically
> controlled by the plug and play OS flag in smbios.
>


What about not PNP Operating Systems? The firmware should address them all.
Also, what about the first time we load the OS, this smbios OS flag would not be there
and we can't install the system. (if I got this flag right)

This seems to me as to much changes to the ecosystem
(this firmware, all pxe drivers out there, specific issues with different OSes...)
for the specific case, I am not saying we shouldn't do that for the long run.

Thanks,
Marcel