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

[Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>]

On Fri, Jan 23, 2026 at 04:42:54PM -0800, Mukesh R wrote:
> 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.
> 

Why not placing the caller side after the function definition then?

Thanks,
Stanislav

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