Re: [PATCH v0 10/15] PCI: hv: Build device id for a VMBus device

Mukesh R
2026-01-24T00:43:03Z

On 1/20/26 14:22, Stanislav Kinsburskii wrote:
> On Mon, Jan 19, 2026 at 10:42:25PM -0800, Mukesh R wrote:
>> From: Mukesh Rathor <mrathor@linux.microsoft.com>
>>
>> On Hyper-V, most hypercalls related to PCI passthru to map/unmap regions,
>> interrupts, etc need a device id as a parameter. This device id refers
>> to that specific device during the lifetime of passthru.
>>
>> An L1VH VM only contains VMBus based devices. A device id for a VMBus
>> device is slightly different in that it uses the hv_pcibus_device info
>> for building it to make sure it matches exactly what the hypervisor
>> expects. This VMBus based device id is needed when attaching devices in
>> an L1VH based guest VM. Before building it, a check is done to make sure
>> the device is a valid VMBus device.
>>
>> Signed-off-by: Mukesh Rathor <mrathor@linux.microsoft.com>
>> ---
>> arch/x86/include/asm/mshyperv.h | 2 ++
>> drivers/pci/controller/pci-hyperv.c | 29 +++++++++++++++++++++++++++++
>> 2 files changed, 31 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
>> index eef4c3a5ba28..0d7fdfb25e76 100644
>> --- a/arch/x86/include/asm/mshyperv.h
>> +++ b/arch/x86/include/asm/mshyperv.h
>> @@ -188,6 +188,8 @@ bool hv_vcpu_is_preempted(int vcpu);
>> static inline void hv_apic_init(void) {}
>> #endif
>>
>> +u64 hv_pci_vmbus_device_id(struct pci_dev *pdev);
>> +
>> struct irq_domain *hv_create_pci_msi_domain(void);
>>
>> int hv_map_msi_interrupt(struct irq_data *data,
>> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
>> index 8bc6a38c9b5a..40f0b06bb966 100644
>> --- a/drivers/pci/controller/pci-hyperv.c
>> +++ b/drivers/pci/controller/pci-hyperv.c
>> @@ -579,6 +579,8 @@ static void hv_pci_onchannelcallback(void *context);
>> #define DELIVERY_MODE APIC_DELIVERY_MODE_FIXED
>> #define HV_MSI_CHIP_FLAGS MSI_CHIP_FLAG_SET_ACK
>>
>> +static bool hv_vmbus_pci_device(struct pci_bus *pbus);
>> +
>
> Why not moving this static function definition above the called instead of
> defining the prototype?

Did you see the function implementation? It has other dependencies that
are later, it would need code reorg.

Thanks,
-Mukesh

>> static int hv_pci_irqchip_init(void)
>> {
>> return 0;
>> @@ -598,6 +600,26 @@ static unsigned int hv_msi_get_int_vector(struct irq_data *data)
>>
>> #define hv_msi_prepare pci_msi_prepare
>>
>> +u64 hv_pci_vmbus_device_id(struct pci_dev *pdev)
>> +{
>> + u64 u64val;
>
> This variable is redundant.

Not really. It helps with debug by putting a quick print, and is
harmless.

>> + struct hv_pcibus_device *hbus;
>> + struct pci_bus *pbus = pdev->bus;
>> +
>> + if (!hv_vmbus_pci_device(pbus))
>> + return 0;
>> +
>> + hbus = container_of(pbus->sysdata, struct hv_pcibus_device, sysdata);
>> + u64val = (hbus->hdev->dev_instance.b[5] << 24) |
>> + (hbus->hdev->dev_instance.b[4] << 16) |
>> + (hbus->hdev->dev_instance.b[7] << 8) |
>> + (hbus->hdev->dev_instance.b[6] & 0xf8) |
>> + PCI_FUNC(pdev->devfn);
>> +
>
> It looks like this value always fits into 32 bit, so what is the value
> in returning 64 bit?

The ABI has device id defined as 64bits where this is assigned.

Thanks,
-Mukesh

> Thanks,
> Stanislav
>
>> + return u64val;
>> +}
>> +EXPORT_SYMBOL_GPL(hv_pci_vmbus_device_id);
>> +
>> /**
>> * hv_irq_retarget_interrupt() - "Unmask" the IRQ by setting its current
>> * affinity.
>> @@ -1404,6 +1426,13 @@ static struct pci_ops hv_pcifront_ops = {
>> .write = hv_pcifront_write_config,
>> };
>>
>> +#ifdef CONFIG_X86
>> +static bool hv_vmbus_pci_device(struct pci_bus *pbus)
>> +{
>> + return pbus->ops == &hv_pcifront_ops;
>> +}
>> +#endif /* CONFIG_X86 */
>> +
>> /*
>> * Paravirtual backchannel
>> *
>> --
>> 2.51.2.vfs.0.1
>>