On 2024/11/1 21:09, Stewart Hildebrand wrote:
> On 10/24/24 05:06, Jiqian Chen wrote:
>> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
>> index 3635d1b39f79..5b10d501d566 100644
>> --- a/hw/xen/xen_pt.c
>> +++ b/hw/xen/xen_pt.c
>> @@ -766,6 +766,50 @@ static void xen_pt_destroy(PCIDevice *d) {
>> }
>> /* init */
>>
>> +#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 42000
>> +static bool xen_pt_need_gsi(void)
>> +{
>> + FILE *fp;
>> + int len;
>> + char type[10];
>
> A brief in-code comment to explain how you arrived at 10 would be
> appreciated.
The max number of characters in the description of the "guest_type" is 4 ("PVH"
plus line break).
I set it to 10 to prevent longer description types in the future.
Do you have another suggest number?
>
>> + const char *guest_type = "/sys/hypervisor/guest_type";
>> +
>> + fp = fopen(guest_type, "r");
>> + if (fp == NULL) {
>> + error_report("Cannot open %s: %s", guest_type, strerror(errno));
>> + return false;
>> + }
>> + fgets(type, sizeof(type), fp);
>
> Please check the return value of fgets.
Will change in next version.
>
>> + fclose(fp);
>> +
>> + len = strlen(type);
>
> Before passing to strlen, is "type" always guaranteed to have a
> terminating '\0' character?
Yes, "fgets" will guarantee that, and I will add check for "fgets" when it
returns NULL in next version.
>
>> + if (len) {
>> + type[len - 1] = '\0';
>> + if (!strcmp(type, "PVH")) {
>> + return true;
>> + }
>> + }
>> + return false;
>> +}
>> +
>> +static int xen_pt_map_pirq_for_gsi(PCIDevice *d, int *pirq)
>> +{
>> + int gsi;
>> + XenPCIPassthroughState *s = XEN_PT_DEVICE(d);
>> +
>> + gsi = xc_pcidev_get_gsi(xen_xc,
>> + PCI_SBDF(s->real_device.domain,
>> + s->real_device.bus,
>> + s->real_device.dev,
>> + s->real_device.func));
>> + if (gsi >= 0) {
>> + return xc_physdev_map_pirq_gsi(xen_xc, xen_domid, gsi, pirq);
>> + }
>> +
>> + return gsi;
>> +}
>> +#endif
>> +
>> static void xen_pt_realize(PCIDevice *d, Error **errp)
>> {
>> ERRP_GUARD();
>> @@ -847,7 +891,16 @@ static void xen_pt_realize(PCIDevice *d, Error **errp)
>> goto out;
>> }
>>
>> +#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 42000
>> + if (xen_pt_need_gsi()) {
>> + rc = xen_pt_map_pirq_for_gsi(d, &pirq);
>> + } else {
>> + rc = xc_physdev_map_pirq(xen_xc, xen_domid, machine_irq, &pirq);
>> + }
>> +#else
>> rc = xc_physdev_map_pirq(xen_xc, xen_domid, machine_irq, &pirq);
>> +#endif
>> +
>> if (rc < 0) {
>> XEN_PT_ERR(d, "Mapping machine irq %u to pirq %i failed, (err:
>> %d)\n",
>> machine_irq, pirq, errno);
>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
>> index eb26cac81098..07805aa8a5f3 100644
>> --- a/include/hw/pci/pci.h
>> +++ b/include/hw/pci/pci.h
>> @@ -23,6 +23,10 @@ extern bool pci_available;
>> #define PCI_SLOT_MAX 32
>> #define PCI_FUNC_MAX 8
>>
>> +#define PCI_SBDF(seg, bus, dev, func) \
>> + ((((uint32_t)(seg)) << 16) | \
>> + (PCI_BUILD_BDF(bus, PCI_DEVFN(dev, func))))
>> +
>> /* Class, Vendor and Device IDs from Linux's pci_ids.h */
>> #include "hw/pci/pci_ids.h"
>>
>
--
Best regards,
Jiqian Chen.
