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

[Thierry Escande <thierry.escande@xxxxxxxxxx>]

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?

> 
>> +    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.

Regards,


--
Thierry Escande | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech