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

[Mukesh R <mrathor@linux.microsoft.com>]

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