Re: [PATCH 03/17] hvmloader: add function to set the emulated machine type (i440/Q35)

[Thierry Escande <thierry.escande@xxxxxxxxxx>]

On 5/4/26 12:58, Jan Beulich wrote:
> On 28.04.2026 12:39, Roger Pau Monné wrote:
>> On Fri, Mar 13, 2026 at 04:35:01PM +0000, Thierry Escande wrote:
>>> --- a/tools/firmware/hvmloader/pci_regs.h
>>> +++ b/tools/firmware/hvmloader/pci_regs.h
>>> @@ -107,6 +107,10 @@
>>>  
>>>  #define PCI_INTEL_OPREGION 0xfc /* 4 bits */
>>>  
>>> +#define PCI_VENDOR_ID_INTEL              0x8086
>>> +#define PCI_DEVICE_ID_INTEL_82441        0x1237
>>> +#define PCI_DEVICE_ID_INTEL_Q35_MCH      0x29c0
>>
>> In Xen we have a separate file for vendor and device IDs, called
>> pci_ids.h.  Maybe it would be better to use a similar approach in
>> hvmloader, and keep pci_regs.h only containing PCI register offsets.
> 
> Can't hvmloader simply re-use Xen's header(s)?

If it's ok to add a symlink to pci_ids.h in tools/include/xen then yes.

>>> +error:
>>> +    printf("Unknown emulated chipset encountered, VID=%04Xh, DID=%04Xh\n",
>>
>> We don't usually use the h suffix in hex numbers in hvmloader, it's
>> more common to prefix them with 0x, so I would recommend to use the %#06x
>> formatter instead.
> 
> I'd generally advise against use of # with a width specifier, as that ends
> up awkward for 0. That is, %#x is fine and generally to be preferred, but
> for a specific with it might better be 0x%0<n>x (with n=4 here). Arguably
> here we don't really expect either of the values to be 0, so the suggested
> use may indeed be okay in this case (while still introducing an example
> which later may be copied elsewhere without much thought).
> 

Indeed printf("%#08x\n", 0x1234) outputs 0x001234. so 0x%04x then.

Regards,


--
Thierry Escande | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech