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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.06.2026 12:01, Thierry Escande wrote:
> 
> 
> On 5/4/26 16:43, Jan Beulich wrote:
>> On 13.03.2026 17:35, Thierry Escande wrote:
>>> @@ -648,6 +649,47 @@ void __bug(const char *file, int line)
>>>      crash();
>>>  }
>>>  
>>> +machine_type_t machine_type;
>>> +
>>> +void init_pc_machine_type(void)
>>> +{
>>> +    uint16_t vendor_id;
>>> +    uint16_t device_id;
>>> +
>>> +    if ( machine_type != MACHINE_TYPE_UNDEFINED )
>>> +        return;
>>> +
>>> +    vendor_id = pci_readw(0, PCI_VENDOR_ID);
>>> +    device_id = pci_readw(0, PCI_DEVICE_ID);
>>> +
>>> +    /* only Intel platforms are emulated currently */
>>
>> Nit: Comment style.
> 
> That's for the missing capital on the first word and full stop, right?

See ./CODING_STYLE - the full stop isn't required here, but the capital first
letter indeed is.

>>> +    if ( vendor_id != PCI_VENDOR_ID_INTEL )
>>> +        goto error;
>>> +
>>> +    switch ( device_id )
>>> +    {
>>> +    case PCI_DEVICE_ID_INTEL_82441:
>>> +        machine_type = MACHINE_TYPE_I440;
>>> +        printf("Detected i440 chipset\n");
>>> +        break;
>>> +
>>> +    case PCI_DEVICE_ID_INTEL_Q35_MCH:
>>> +        machine_type = MACHINE_TYPE_Q35;
>>> +        printf("Detected Q35 chipset\n");
>>> +        break;
>>> +
>>> +    default:
>>> +        goto error;
>>> +    }
>>> +
>>> +    return;
>>> +
>>> +error:
>>
>> Nit: Labels indented by at least one blank please.
>>
>>> +    printf("Unknown emulated chipset encountered, VID=%04Xh, DID=%04Xh\n",
>>> +           vendor_id, device_id);
>>> +    BUG();
>>
>> Can't this be moved up into the default case, thus avoiding "goto" and label
>> altogether?
> 
> There is already a goto error on the vendor ID check. That makes it more
> readable imo.

Hmm, maybe to some people. To me the extra "return" goes against better
readability. I agree we don't want to duplicate things, but we can still get
away with one less "goto" if the label was moved next to the "default" one
in the switch().

Jan