Re: [PATCH v2 6/6] hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE

[Chuck Zmudzinski]

On 1/6/23 6:04 PM, Chuck Zmudzinski wrote:
> On 1/6/23 2:08 PM, Chuck Zmudzinski wrote:
>> On 1/6/23 7:25 AM, Philippe Mathieu-Daudé wrote:
>>> On 6/1/23 12:57, Bernhard Beschow wrote:
>>>> 
>>>> 
>>>> Am 4. Januar 2023 15:35:33 UTC schrieb "Philippe Mathieu-Daudé" 
>>>> <philmd@linaro.org>:
>>>>> +Markus/Thomas
>>>>>
>>>>> On 4/1/23 15:44, Bernhard Beschow wrote:
>>>>>> During the last patches, TYPE_PIIX3_XEN_DEVICE turned into a clone of
>>>>>> TYPE_PIIX3_DEVICE. Remove this redundancy.
>>>>>>
>>>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>>>> ---
>>>>>>    hw/i386/pc_piix.c             |  4 +---
>>>>>>    hw/isa/piix.c                 | 20 --------------------
>>>>>>    include/hw/southbridge/piix.h |  1 -
>>>>>>    3 files changed, 1 insertion(+), 24 deletions(-)
>>> 
>>> 
>>>>>>    -static void piix3_xen_class_init(ObjectClass *klass, void *data)
>>>>>> -{
>>>>>> -    DeviceClass *dc = DEVICE_CLASS(klass);
>>>>>> -    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>>>>>> -
>>>>>> -    k->realize = piix3_realize;
>>>>>> -    /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
>>>>>> -    k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0;
>>>>>> -    dc->vmsd = &vmstate_piix3;
>>>>>
>>>>> IIUC, since this device is user-creatable, we can't simply remove it
>>>>> without going thru the deprecation process.
>>>> 
>>>> AFAICS this device is actually not user-creatable since dc->user_creatable 
>>>> is set to false once in the base class. I think it is safe to remove the 
>>>> Xen class unless there are ABI issues.
>>> Great news!
>> 
>> I don't know if this means the device is user-creatable:
>> 
>> chuckz@bullseye:~$ qemu-system-i386 -device piix3-ide-xen,help
>> piix3-ide-xen options:
>>   addr=<int32>           - Slot and optional function number, example: 06.0 
>> or 06 (default: -1)
>>   failover_pair_id=<str>
>>   multifunction=<bool>   - on/off (default: false)
>>   rombar=<uint32>        -  (default: 1)
>>   romfile=<str>
>>   x-pcie-extcap-init=<bool> - on/off (default: true)
>>   x-pcie-lnksta-dllla=<bool> - on/off (default: true)
>> 
>> Today I am running qemu-5.2 on Debian 11, so this output is for
>> qemu 5.2, and that version of qemu has a piix3-ide-xen device.
>> Is that this same device that is being removed? If so, it seems to
>> me that at least as of qemu 5.2, the device was user-creatable.
>> 
>> Chuck
> 
> Good news! It looks the device was removed as user-creatable since version 
> 5.2:
> 
> chuckz@debian:~$ qemu-system-i386-7.50 -device help | grep piix
> name "piix3-usb-uhci", bus PCI
> name "piix4-usb-uhci", bus PCI
> name "piix3-ide", bus PCI
> name "piix4-ide", bus PCI
> chuckz@debian:~$ qemu-system-i386-7.50-bernhard-v2 -device help | grep piix
> name "piix3-usb-uhci", bus PCI
> name "piix4-usb-uhci", bus PCI
> name "piix3-ide", bus PCI
> name "piix4-ide", bus PCI
> chuckz@debian:~$
> 
> The piix3-ide-xen device is not present either with or without Bernhard's 
> patches
> for current qemu 7.50, the development version for qemu 8.0
> 
> Cheers,
> 
> Chuck


I traced where the pciix3-ide-xen device was removed:

It was 7851b21a81 (hw/ide/piix: Remove redundant "piix3-ide-xen" device class)

https://gitlab.com/qemu-project/qemu/-/commit/7851b21a8192750adecbcf6e8780a20de5891ad6

about six months ago. That was between 7.0 and 7.1. So the device being removed
here is definitely not user-creatable, but it appears that this piix3-ide-xen
device that was removed between 7.0 and 7.1 was user-creatable. Does that one
need to go through the deprecation process? Or, since no one has complained
it is gone, maybe we don't need to worry about it?

Cheers,

Chuck