On 6/26/25 4:05 AM, Shaoqin Huang wrote:
> Hi Eric,
>
> On 6/23/25 5:20 PM, Eric Auger wrote:
>>
>>
>> On 6/17/25 5:05 AM, Shaoqin Huang wrote:
>>> Set the "use-legacy-x86-rom" property to false by default, and only set
>>> it to true on x86 since only x86 will need it.
>> s/compatatibility/compatibility in the title
>
> Ok. Will fix it.
>
>>>
>>> Signed-off-by: Shaoqin Huang <shahu...@redhat.com>
>>> ---
>>> hw/display/ramfb-standalone.c | 2 +-
>>> hw/i386/pc_q35.c | 3 +++
>>> hw/vfio/pci.c | 2 +-
>>> 3 files changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/display/ramfb-standalone.c b/hw/display/ramfb-
>>> standalone.c
>>> index af1175bf96..ddbf42f181 100644
>>> --- a/hw/display/ramfb-standalone.c
>>> +++ b/hw/display/ramfb-standalone.c
>>> @@ -63,7 +63,7 @@ static const VMStateDescription ramfb_dev_vmstate = {
>>> static const Property ramfb_properties[] = {
>>> DEFINE_PROP_BOOL("x-migrate", RAMFBStandaloneState, migrate,
>>> true),
>>> - DEFINE_PROP_BOOL("use-legacy-x86-rom", RAMFBStandaloneState,
>>> use_legacy_x86_rom, true),
>>> + DEFINE_PROP_BOOL("use-legacy-x86-rom", RAMFBStandaloneState,
>>> use_legacy_x86_rom, false),
>>> };
>>> static void ramfb_class_initfn(ObjectClass *klass, void *data)
>>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>>> index fd96d0345c..f6d89578d0 100644
>>> --- a/hw/i386/pc_q35.c
>>> +++ b/hw/i386/pc_q35.c
>>> @@ -45,6 +45,7 @@
>>> #include "hw/i386/pc.h"
>>> #include "hw/i386/amd_iommu.h"
>>> #include "hw/i386/intel_iommu.h"
>>> +#include "hw/vfio/pci.h"
>>> #include "hw/virtio/virtio-iommu.h"
>>> #include "hw/display/ramfb.h"
>>> #include "hw/ide/pci.h"
>>> @@ -67,6 +68,8 @@
>>> static GlobalProperty pc_q35_compat_defaults[] = {
>>> { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "39" },
>>> + { TYPE_RAMFB_DEVICE, "use-legacy-x86-rom", "true" },
>>> + { TYPE_VFIO_PCI, "use-legacy-x86-rom", "true" },
>> this will only keep the legacy behavior along with q35 machine type but
>> not on other machines being used for x86. what about pc-i440fx? Doesn't
>> it apply to it as well? Are there other machine types also impacted.
>
> Ok I will also add it with pc-i440fx. I think only q35 and i440fx are
> impacted.
>
>>
>> Also what about Daniel's comment in v3:
>> https://lore.kernel.org/all/aeak8utppkhep...@redhat.com/
>> "For non-x86, historical versioned machine types will need
>> likely it set to true, in order to avoid the memory layout
>> being changed IIUC."
>>
>> Is it actually needed?
>
> If those machine types need to set it to true. I think they can set it
> after they have this property.
nope it does not work like that. In case we really need to take care of
this, this must be handled by compats.
Thanks
Eric
>
> Thanks,
> Shaoqin
>
>>
>> Thanks
>>
>> Eric
>>> };
>>> static const size_t pc_q35_compat_defaults_len =
>>> G_N_ELEMENTS(pc_q35_compat_defaults);
>>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>>> index ff0d93fae0..a529500b70 100644
>>> --- a/hw/vfio/pci.c
>>> +++ b/hw/vfio/pci.c
>>> @@ -3564,7 +3564,7 @@ static const TypeInfo vfio_pci_dev_info = {
>>> static const Property vfio_pci_dev_nohotplug_properties[] = {
>>> DEFINE_PROP_BOOL("ramfb", VFIOPCIDevice, enable_ramfb, false),
>>> - DEFINE_PROP_BOOL("use-legacy-x86-rom", VFIOPCIDevice,
>>> use_legacy_x86_rom, true),
>>> + DEFINE_PROP_BOOL("use-legacy-x86-rom", VFIOPCIDevice,
>>> use_legacy_x86_rom, false),
>>> DEFINE_PROP_ON_OFF_AUTO("x-ramfb-migrate", VFIOPCIDevice,
>>> ramfb_migrate,
>>> ON_OFF_AUTO_AUTO),
>>> };
>>
>
