Re: [Xen-devel] [RFC PATCH 3/4] xen-pt: bind/unbind interrupt remapping format MSI
On Thu, Mar 30, 2017 at 05:51:45PM +0100, Anthony PERARD wrote: >On Fri, Mar 17, 2017 at 07:29:16PM +0800, Lan Tianyu wrote: >> From: Chao Gao >> >> If a vIOMMU is exposed to guest, guest will configure the msi to remapping >> format. The original code isn't suitable to the new format. A new pair >> bind/unbind interfaces are added for this usage. This patch recognizes >> this case and use new interfaces to bind/unbind msi. >> >> Signed-off-by: Chao Gao >> Signed-off-by: Lan Tianyu >> --- >> hw/xen/xen_pt_msi.c | 36 >> include/hw/i386/apic-msidef.h | 1 + >> 2 files changed, 29 insertions(+), 8 deletions(-) >> >> diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c >> index 62add06..8b0d7fc 100644 >> --- a/hw/xen/xen_pt_msi.c >> +++ b/hw/xen/xen_pt_msi.c >> @@ -161,6 +161,7 @@ static int msi_msix_update(XenPCIPassthroughState *s, >> uint8_t gvec = msi_vector(data); >> uint32_t gflags = msi_gflags(data, addr); >> int rc = 0; >> +bool ir = !!(addr & MSI_ADDR_IM_MASK); >> uint64_t table_addr = 0; >> >> XEN_PT_LOG(d, "Updating MSI%s with pirq %d gvec %#x gflags %#x" >> @@ -171,8 +172,14 @@ static int msi_msix_update(XenPCIPassthroughState *s, >> table_addr = s->msix->mmio_base_addr; >> } >> >> -rc = xc_domain_update_msi_irq(xen_xc, xen_domid, gvec, >> +if (ir) { > >You could maybe use add&MSI_ADDR_IM_MASK instead of going through a >variable. > >> +rc = xc_domain_update_msi_irq_remapping(xen_xc, xen_domid, pirq, >> +d->devfn, data, addr, table_addr); > >Do you also want to update the XEN_PT_LOG above? Since it does not >always reflect the update_msi call anymore. Yes. I adjust the output. > >> +} >> +else { >> +rc = xc_domain_update_msi_irq(xen_xc, xen_domid, gvec, >>pirq, gflags, table_addr); >> +} >> >> if (rc) { >> XEN_PT_ERR(d, "Updating of MSI%s failed. (err: %d)\n", >> @@ -204,13 +211,26 @@ static int msi_msix_disable(XenPCIPassthroughState *s, >> } >> >> if (is_binded) { >> -XEN_PT_LOG(d, "Unbind MSI%s with pirq %d, gvec %#x\n", >> - is_msix ? "-X" : "", pirq, gvec); >> -rc = xc_domain_unbind_msi_irq(xen_xc, xen_domid, gvec, pirq, >> gflags); >> -if (rc) { >> -XEN_PT_ERR(d, "Unbinding of MSI%s failed. (err: %d, pirq: %d, >> gvec: %#x)\n", >> - is_msix ? "-X" : "", errno, pirq, gvec); >> -return rc; >> +if ( addr & MSI_ADDR_IM_MASK ) { >> +XEN_PT_LOG(d, "Unbinding of MSI%s . ( pirq: %d, data: %x, addr: >> %lx)\n", > >For addr, it should be PRIx64 instead of %lx. > >> + is_msix ? "-X" : "", pirq, data, addr); >> +rc = xc_domain_unbind_msi_irq_remapping(xen_xc, xen_domid, pirq, >> +d->devfn, data, addr); >> +if (rc) { >> +XEN_PT_ERR(d, "Unbinding of MSI%s . (error: %d, pirq: %d, >> data: %x, addr: %lx)\n", >> + is_msix ? "-X" : "", rc, pirq, data, addr); >> +return rc; >> +} >> + >> +} else { >> +XEN_PT_LOG(d, "Unbind MSI%s with pirq %d, gvec %#x\n", >> + is_msix ? "-X" : "", pirq, gvec); >> +rc = xc_domain_unbind_msi_irq(xen_xc, xen_domid, gvec, pirq, >> gflags); >> +if (rc) { >> +XEN_PT_ERR(d, "Unbinding of MSI%s failed. (err: %d, pirq: >> %d, gvec: %#x)\n", >> + is_msix ? "-X" : "", errno, pirq, gvec); >> +return rc; >> +} >> } >> } >> >> diff --git a/include/hw/i386/apic-msidef.h b/include/hw/i386/apic-msidef.h >> index 8b4d4cc..08b584f 100644 >> --- a/include/hw/i386/apic-msidef.h >> +++ b/include/hw/i386/apic-msidef.h >> @@ -27,5 +27,6 @@ >> #define MSI_ADDR_DEST_ID_SHIFT 12 >> #define MSI_ADDR_DEST_IDX_SHIFT 4 >> #define MSI_ADDR_DEST_ID_MASK 0x000 > >Could you add a 0 to dest_id here? So their will be 8 digit and it those >not look weird when compared to the next define. > Will do. >> +#define MSI_ADDR_IM_MASK 0x0010 > >Is the definition of MSI_ADDR_IM_MASK available somewhere? In the Intel >SDM I've only found this bit to be reserved. Yes, it is defined in VT-d spec 5.1.5.2 MSI and MSI-X Register Programming. I made a mistake here. I should use MSI_ADDR_IF_MASK. Thanks Chao > >Thanks, > >-- >Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH 3/4] xen-pt: bind/unbind interrupt remapping format MSI
On Fri, Mar 17, 2017 at 07:29:16PM +0800, Lan Tianyu wrote: > From: Chao Gao > > If a vIOMMU is exposed to guest, guest will configure the msi to remapping > format. The original code isn't suitable to the new format. A new pair > bind/unbind interfaces are added for this usage. This patch recognizes > this case and use new interfaces to bind/unbind msi. > > Signed-off-by: Chao Gao > Signed-off-by: Lan Tianyu > --- > hw/xen/xen_pt_msi.c | 36 > include/hw/i386/apic-msidef.h | 1 + > 2 files changed, 29 insertions(+), 8 deletions(-) > > diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c > index 62add06..8b0d7fc 100644 > --- a/hw/xen/xen_pt_msi.c > +++ b/hw/xen/xen_pt_msi.c > @@ -161,6 +161,7 @@ static int msi_msix_update(XenPCIPassthroughState *s, > uint8_t gvec = msi_vector(data); > uint32_t gflags = msi_gflags(data, addr); > int rc = 0; > +bool ir = !!(addr & MSI_ADDR_IM_MASK); > uint64_t table_addr = 0; > > XEN_PT_LOG(d, "Updating MSI%s with pirq %d gvec %#x gflags %#x" > @@ -171,8 +172,14 @@ static int msi_msix_update(XenPCIPassthroughState *s, > table_addr = s->msix->mmio_base_addr; > } > > -rc = xc_domain_update_msi_irq(xen_xc, xen_domid, gvec, > +if (ir) { You could maybe use add&MSI_ADDR_IM_MASK instead of going through a variable. > +rc = xc_domain_update_msi_irq_remapping(xen_xc, xen_domid, pirq, > +d->devfn, data, addr, table_addr); Do you also want to update the XEN_PT_LOG above? Since it does not always reflect the update_msi call anymore. > +} > +else { > +rc = xc_domain_update_msi_irq(xen_xc, xen_domid, gvec, >pirq, gflags, table_addr); > +} > > if (rc) { > XEN_PT_ERR(d, "Updating of MSI%s failed. (err: %d)\n", > @@ -204,13 +211,26 @@ static int msi_msix_disable(XenPCIPassthroughState *s, > } > > if (is_binded) { > -XEN_PT_LOG(d, "Unbind MSI%s with pirq %d, gvec %#x\n", > - is_msix ? "-X" : "", pirq, gvec); > -rc = xc_domain_unbind_msi_irq(xen_xc, xen_domid, gvec, pirq, gflags); > -if (rc) { > -XEN_PT_ERR(d, "Unbinding of MSI%s failed. (err: %d, pirq: %d, > gvec: %#x)\n", > - is_msix ? "-X" : "", errno, pirq, gvec); > -return rc; > +if ( addr & MSI_ADDR_IM_MASK ) { > +XEN_PT_LOG(d, "Unbinding of MSI%s . ( pirq: %d, data: %x, addr: > %lx)\n", For addr, it should be PRIx64 instead of %lx. > + is_msix ? "-X" : "", pirq, data, addr); > +rc = xc_domain_unbind_msi_irq_remapping(xen_xc, xen_domid, pirq, > +d->devfn, data, addr); > +if (rc) { > +XEN_PT_ERR(d, "Unbinding of MSI%s . (error: %d, pirq: %d, > data: %x, addr: %lx)\n", > + is_msix ? "-X" : "", rc, pirq, data, addr); > +return rc; > +} > + > +} else { > +XEN_PT_LOG(d, "Unbind MSI%s with pirq %d, gvec %#x\n", > + is_msix ? "-X" : "", pirq, gvec); > +rc = xc_domain_unbind_msi_irq(xen_xc, xen_domid, gvec, pirq, > gflags); > +if (rc) { > +XEN_PT_ERR(d, "Unbinding of MSI%s failed. (err: %d, pirq: > %d, gvec: %#x)\n", > + is_msix ? "-X" : "", errno, pirq, gvec); > +return rc; > +} > } > } > > diff --git a/include/hw/i386/apic-msidef.h b/include/hw/i386/apic-msidef.h > index 8b4d4cc..08b584f 100644 > --- a/include/hw/i386/apic-msidef.h > +++ b/include/hw/i386/apic-msidef.h > @@ -27,5 +27,6 @@ > #define MSI_ADDR_DEST_ID_SHIFT 12 > #define MSI_ADDR_DEST_IDX_SHIFT 4 > #define MSI_ADDR_DEST_ID_MASK 0x000 Could you add a 0 to dest_id here? So their will be 8 digit and it those not look weird when compared to the next define. > +#define MSI_ADDR_IM_MASK 0x0010 Is the definition of MSI_ADDR_IM_MASK available somewhere? In the Intel SDM I've only found this bit to be reserved. Thanks, -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [RFC PATCH 3/4] xen-pt: bind/unbind interrupt remapping format MSI
From: Chao Gao If a vIOMMU is exposed to guest, guest will configure the msi to remapping format. The original code isn't suitable to the new format. A new pair bind/unbind interfaces are added for this usage. This patch recognizes this case and use new interfaces to bind/unbind msi. Signed-off-by: Chao Gao Signed-off-by: Lan Tianyu --- hw/xen/xen_pt_msi.c | 36 include/hw/i386/apic-msidef.h | 1 + 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c index 62add06..8b0d7fc 100644 --- a/hw/xen/xen_pt_msi.c +++ b/hw/xen/xen_pt_msi.c @@ -161,6 +161,7 @@ static int msi_msix_update(XenPCIPassthroughState *s, uint8_t gvec = msi_vector(data); uint32_t gflags = msi_gflags(data, addr); int rc = 0; +bool ir = !!(addr & MSI_ADDR_IM_MASK); uint64_t table_addr = 0; XEN_PT_LOG(d, "Updating MSI%s with pirq %d gvec %#x gflags %#x" @@ -171,8 +172,14 @@ static int msi_msix_update(XenPCIPassthroughState *s, table_addr = s->msix->mmio_base_addr; } -rc = xc_domain_update_msi_irq(xen_xc, xen_domid, gvec, +if (ir) { +rc = xc_domain_update_msi_irq_remapping(xen_xc, xen_domid, pirq, +d->devfn, data, addr, table_addr); +} +else { +rc = xc_domain_update_msi_irq(xen_xc, xen_domid, gvec, pirq, gflags, table_addr); +} if (rc) { XEN_PT_ERR(d, "Updating of MSI%s failed. (err: %d)\n", @@ -204,13 +211,26 @@ static int msi_msix_disable(XenPCIPassthroughState *s, } if (is_binded) { -XEN_PT_LOG(d, "Unbind MSI%s with pirq %d, gvec %#x\n", - is_msix ? "-X" : "", pirq, gvec); -rc = xc_domain_unbind_msi_irq(xen_xc, xen_domid, gvec, pirq, gflags); -if (rc) { -XEN_PT_ERR(d, "Unbinding of MSI%s failed. (err: %d, pirq: %d, gvec: %#x)\n", - is_msix ? "-X" : "", errno, pirq, gvec); -return rc; +if ( addr & MSI_ADDR_IM_MASK ) { +XEN_PT_LOG(d, "Unbinding of MSI%s . ( pirq: %d, data: %x, addr: %lx)\n", + is_msix ? "-X" : "", pirq, data, addr); +rc = xc_domain_unbind_msi_irq_remapping(xen_xc, xen_domid, pirq, +d->devfn, data, addr); +if (rc) { +XEN_PT_ERR(d, "Unbinding of MSI%s . (error: %d, pirq: %d, data: %x, addr: %lx)\n", + is_msix ? "-X" : "", rc, pirq, data, addr); +return rc; +} + +} else { +XEN_PT_LOG(d, "Unbind MSI%s with pirq %d, gvec %#x\n", + is_msix ? "-X" : "", pirq, gvec); +rc = xc_domain_unbind_msi_irq(xen_xc, xen_domid, gvec, pirq, gflags); +if (rc) { +XEN_PT_ERR(d, "Unbinding of MSI%s failed. (err: %d, pirq: %d, gvec: %#x)\n", + is_msix ? "-X" : "", errno, pirq, gvec); +return rc; +} } } diff --git a/include/hw/i386/apic-msidef.h b/include/hw/i386/apic-msidef.h index 8b4d4cc..08b584f 100644 --- a/include/hw/i386/apic-msidef.h +++ b/include/hw/i386/apic-msidef.h @@ -27,5 +27,6 @@ #define MSI_ADDR_DEST_ID_SHIFT 12 #define MSI_ADDR_DEST_IDX_SHIFT 4 #define MSI_ADDR_DEST_ID_MASK 0x000 +#define MSI_ADDR_IM_MASK 0x0010 #endif /* HW_APIC_MSIDEF_H */ -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel