| Message ID | 20200210033953.99692-4-boqun.feng@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: hv: Generify pci-hyperv.c | expand |
> From: linux-hyperv-owner@vger.kernel.org > <linux-hyperv-owner@vger.kernel.org> On Behalf Of Boqun Feng > diff --git a/arch/x86/include/asm/hyperv-tlfs.h > b/arch/x86/include/asm/hyperv-tlfs.h > > +union hv_msi_entry { > + u64 as_uint64; > + struct { > + u32 address; > + u32 data; > + } __packed; > +}; Just a small thing: should we move the __packed to after the "}" of the union hv_msi_entry ? Reviewed-by: Dexuan Cui <decui@microsoft.com>
On Thu, Feb 13, 2020 at 04:18:01AM +0000, Dexuan Cui wrote: > > From: linux-hyperv-owner@vger.kernel.org > > <linux-hyperv-owner@vger.kernel.org> On Behalf Of Boqun Feng > > diff --git a/arch/x86/include/asm/hyperv-tlfs.h > > b/arch/x86/include/asm/hyperv-tlfs.h > > > > +union hv_msi_entry { > > + u64 as_uint64; > > + struct { > > + u32 address; > > + u32 data; > > + } __packed; > > +}; > > Just a small thing: should we move the __packed to after the "}" of > the union hv_msi_entry ? > Actually, in TLFS header, it's common to put the "__packed" inside the union, rather than after the union. It makes sense because union is different than struct: the alignment requirement of a union is already decided by the "as_*" member, so no need for "__packed" attribute. > Reviewed-by: Dexuan Cui <decui@microsoft.com> Thanks! Regards, Boqun
> From: Boqun Feng <boqun.feng@gmail.com> > Sent: Wednesday, February 12, 2020 11:14 PM > > On Thu, Feb 13, 2020 at 04:18:01AM +0000, Dexuan Cui wrote: > > > From: linux-hyperv-owner@vger.kernel.org > > > <linux-hyperv-owner@vger.kernel.org> On Behalf Of Boqun Feng > > > diff --git a/arch/x86/include/asm/hyperv-tlfs.h > > > b/arch/x86/include/asm/hyperv-tlfs.h > > > > > > +union hv_msi_entry { > > > + u64 as_uint64; > > > + struct { > > > + u32 address; > > > + u32 data; > > > + } __packed; > > > +}; > > > > Just a small thing: should we move the __packed to after the "}" of > > the union hv_msi_entry ? > > > > Actually, in TLFS header, it's common to put the "__packed" inside the > union, rather than after the union. It makes sense because union is > different than struct: the alignment requirement of a union is already > decided by the "as_*" member, so no need for "__packed" attribute. > > Regards, > Boqun I see. Thanks for the explanation! Thanks, -- Dexuan
diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h index a0b6a88d2f05..29336574d0bc 100644 --- a/arch/x86/include/asm/hyperv-tlfs.h +++ b/arch/x86/include/asm/hyperv-tlfs.h @@ -913,11 +913,18 @@ struct hv_partition_assist_pg { u32 tlb_lock_count; }; +union hv_msi_entry { + u64 as_uint64; + struct { + u32 address; + u32 data; + } __packed; +}; + struct hv_interrupt_entry { u32 source; /* 1 for MSI(-X) */ u32 reserved1; - u32 address; - u32 data; + union hv_msi_entry msi_entry; } __packed; /* diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h index 6b79515abb82..81fc30240122 100644 --- a/arch/x86/include/asm/mshyperv.h +++ b/arch/x86/include/asm/mshyperv.h @@ -4,6 +4,7 @@ #include <linux/types.h> #include <linux/nmi.h> +#include <linux/msi.h> #include <asm/io.h> #include <asm/hyperv-tlfs.h> #include <asm/nospec-branch.h> @@ -240,6 +241,13 @@ bool hv_vcpu_is_preempted(int vcpu); static inline void hv_apic_init(void) {} #endif +static inline void hv_set_msi_entry_from_desc(union hv_msi_entry *msi_entry, + struct msi_desc *msi_desc) +{ + msi_entry->address = msi_desc->msg.address_lo; + msi_entry->data = msi_desc->msg.data; +} + #else /* CONFIG_HYPERV */ static inline void hyperv_init(void) {} static inline void hyperv_setup_mmu_ops(void) {} diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c index 0d9b74503577..3f9b220c23ec 100644 --- a/drivers/pci/controller/pci-hyperv.c +++ b/drivers/pci/controller/pci-hyperv.c @@ -1170,8 +1170,7 @@ static void hv_irq_unmask(struct irq_data *data) memset(params, 0, sizeof(*params)); params->partition_id = HV_PARTITION_ID_SELF; params->int_entry.source = 1; /* MSI(-X) */ - params->int_entry.address = msi_desc->msg.address_lo; - params->int_entry.data = msi_desc->msg.data; + hv_set_msi_entry_from_desc(¶ms->int_entry.msi_entry, msi_desc); params->device_id = (hbus->hdev->dev_instance.b[5] << 24) | (hbus->hdev->dev_instance.b[4] << 16) | (hbus->hdev->dev_instance.b[7] << 8) |
Add a new structure (hv_msi_entry), which is also defined in the TLFS, to describe the msi entry for HVCALL_RETARGET_INTERRUPT. The structure is needed because its layout may be different from architecture to architecture. Also add a new generic interface hv_set_msi_entry_from_desc() to allow different archs to set the msi entry from msi_desc. No functional change, only preparation for the future support of virtual PCI on non-x86 architectures. Signed-off-by: Boqun Feng (Microsoft) <boqun.feng@gmail.com> --- arch/x86/include/asm/hyperv-tlfs.h | 11 +++++++++-- arch/x86/include/asm/mshyperv.h | 8 ++++++++ drivers/pci/controller/pci-hyperv.c | 3 +-- 3 files changed, 18 insertions(+), 4 deletions(-)