Re: [PATCH] pci: add romsize property

[David Edmondson]

On Tuesday, 2021-01-19 at 17:51:32 +01, Philippe Mathieu-Daudé wrote:

> +pflash
>
> On 12/18/20 7:27 PM, Paolo Bonzini wrote:
>> This property can be useful for distros to set up known-good ROM sizes for
>> migration purposes.  The VM will fail to start if the ROM is too large,
>> and migration compatibility will not be broken if the ROM is too small.
>> 
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  hw/pci/pci.c             | 19 +++++++++++++++++--
>>  hw/xen/xen_pt_load_rom.c | 14 ++++++++++++--
>>  include/hw/pci/pci.h     |  1 +
>>  3 files changed, 30 insertions(+), 4 deletions(-)
>> 
>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>> index d4349ea577..fd25253c2a 100644
>> --- a/hw/pci/pci.c
>> +++ b/hw/pci/pci.c
>> @@ -67,6 +67,7 @@ static void pcibus_reset(BusState *qbus);
>>  static Property pci_props[] = {
>>      DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
>>      DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
>> +    DEFINE_PROP_UINT32("romsize", PCIDevice, romsize, -1),
>>      DEFINE_PROP_UINT32("rombar",  PCIDevice, rom_bar, 1),
>>      DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present,
>>                      QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false),
>> @@ -2106,6 +2107,11 @@ static void pci_qdev_realize(DeviceState *qdev, Error 
>> **errp)
>>      bool is_default_rom;
>>      uint16_t class_id;
>>  
>> +    if (pci_dev->romsize != -1 && !is_power_of_2(pci_dev->romsize)) {
>> +        error_setg(errp, "ROM size %d is not a power of two", 
>> pci_dev->romsize);
>> +        return;
>> +    }
>
> Some cloud providers already complained the pow2 check in the pflash
> device (wasting host storage). Personally I find using pow2 easier
> and safer.
>
> The pow2 check looks like a separate change however, maybe add in a
> separate patch? Or maybe not :)

Even for flash, padding a read-only device seems straightforward.

For a writable device, is it assumed that a write into the padding
should extend the file?

(I realise that this patch is just for the ROM.)

dme.
-- 
You bring light in.