Re: [PATCH 04/14] powerpc/vas: Setup IRQ mapping and register port for each window

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2019-12-18 at 15:13 -0800, Haren Myneni wrote:
> On Wed, 2019-12-18 at 18:18 +1100, Oliver O'Halloran wrote:
> > On Wed, Nov 27, 2019 at 12:07 PM Haren Myneni <haren@xxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > *snip*
> > >
> > > @@ -36,7 +62,18 @@ static int init_vas_instance(struct platform_device *pdev)
> > >                 return -ENODEV;
> > >         }
> > >
> > > -       if (pdev->num_resources != 4) {
> > > +       rc = of_property_read_u64(dn, "ibm,vas-port", &port);
> > > +       if (rc) {
> > > +               pr_err("No ibm,vas-port property for %s?\n", pdev->name);
> > > +               /* No interrupts property */
> > > +               nresources = 4;
> > > +       }
> > > +
> > > +       /*
> > > +        * interrupts property is available with 'ibm,vas-port' property.
> > > +        * 4 Resources and 1 IRQ if interrupts property is available.
> > > +        */
> > > +       if (pdev->num_resources != nresources) {
> > >                 pr_err("Unexpected DT configuration for [%s, %d]\n",
> > >                                 pdev->name, vasid);
> > >                 return -ENODEV;
> > 
> > Right, so adding the IRQ in firmware will break the VAS driver in
> > existing kernels since it changes the resource count. This is IMO a
> > bug in the VAS driver that you should fix, but it does mean we need to
> > think twice about having firmware assign an interrupt at boot.
> 
> Correct, Hence added vas-user-space nvram switch in skiboot.  
> 
> > 
> > I had a closer look at this series and I'm not convinced that any
> > firmware changes are actually required either. We already have OPAL
> > calls for allocating an hwirq for the kernel to use and for getting
> > the IRQ's XIVE trigger port (see pnv_ocxl_alloc_xive_irq() for an
> > example). Why not use those here too? Doing so would allow us to
> > assign interrupts to individual windows too which might be useful for
> > the windows used by the kernel.
> 
> Thanks for the pointer. like using pnv_ocxl_alloc_xive_irq(), we can
> disregard FW change. BTW, VAS fault handling is needed only for user
> space VAS windows. 
> 
>  int vas_alloc_xive_irq(u32 chipid, u32 *irq, u64 *trigger_addr)
> {
>         __be64 flags, trigger_page;
>         u32 hwirq;
>         s64 rc;
> 
>         hwirq = opal_xive_allocate_irq_raw(chipid);
>         if (hwirq < 0)
>                 return -ENOENT;
> 
>         rc = opal_xive_get_irq_info(hwirq, &flags, NULL, &trigger_page,
> NULL,
>                                 NULL);
>         if (rc || !trigger_page) {
>                 xive_native_free_irq(hwirq);
>                 return -ENOENT;
>         }
> 
>         *irq = hwirq;
>         *trigger_addr = be64_to_cpu(trigger_page);
>         return 0;
> }
> 
> We can have common function for VAS and cxl except per chip IRQ
> allocation is needed for each VAS instance. I will post patch-set with
> this change.
> 

power9 will have only XIVE interrupt controller including on open-power
systems. Correct?

VAS need per chip IRQ allocation. The current interfaces (ex:
xive_native_alloc_irq(void)) allocates IRQ on any chip
(OPAL_XIVE_ANY_CHIP)
So to use these interfaces for VAS, any concerns with the following
patch:
Changes: passing chip_id to xive_native_alloc_irq() and define
xive_native_alloc_get_irq_info() in xive/native.c which can be used in
ocxl and VAS.

diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
index 24cdf97..b310062 100644
--- a/arch/powerpc/include/asm/xive.h
+++ b/arch/powerpc/include/asm/xive.h
@@ -108,7 +108,7 @@ struct xive_q {
 extern int xive_native_populate_irq_data(u32 hw_irq,
 					 struct xive_irq_data *data);
 extern void xive_cleanup_irq_data(struct xive_irq_data *xd);
-extern u32 xive_native_alloc_irq(void);
+extern u32 xive_native_alloc_irq(u32 chip_id);
 extern void xive_native_free_irq(u32 irq);
 extern int xive_native_configure_irq(u32 hw_irq, u32 target, u8 prio, u32 sw_irq);
 
@@ -137,7 +137,8 @@ extern int xive_native_set_queue_state(u32 vp_id, uint32_t prio, u32 qtoggle,
 				       u32 qindex);
 extern int xive_native_get_vp_state(u32 vp_id, u64 *out_state);
 extern bool xive_native_has_queue_state_support(void);
-
+extern int xive_native_alloc_get_irq_info(u32 chip_id, u32 *irq,
+					u64 *trigger_addr);
 #else
 
 static inline bool xive_enabled(void) { return false; }
diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index 66858b7..59009e1 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -1299,7 +1299,7 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
 	vcpu->arch.xive_cam_word = cpu_to_be32(xc->vp_cam | TM_QW1W2_VO);
 
 	/* Allocate IPI */
-	xc->vp_ipi = xive_native_alloc_irq();
+	xc->vp_ipi = xive_native_alloc_irq(OPAL_XIVE_ANY_CHIP);
 	if (!xc->vp_ipi) {
 		pr_err("Failed to allocate xive irq for VCPU IPI\n");
 		r = -EIO;
@@ -1711,7 +1711,7 @@ static int xive_set_source(struct kvmppc_xive *xive, long irq, u64 addr)
 	 * one and get the corresponding data
 	 */
 	if (!state->ipi_number) {
-		state->ipi_number = xive_native_alloc_irq();
+		state->ipi_number = xive_native_alloc_irq(OPAL_XIVE_ANY_CHIP);
 		if (state->ipi_number == 0) {
 			pr_devel("Failed to allocate IPI !\n");
 			return -ENOMEM;
diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
index d83adb1..0adb228 100644
--- a/arch/powerpc/kvm/book3s_xive_native.c
+++ b/arch/powerpc/kvm/book3s_xive_native.c
@@ -359,7 +359,7 @@ static int kvmppc_xive_native_set_source(struct kvmppc_xive *xive, long irq,
 	 * one and get the corresponding data
 	 */
 	if (!state->ipi_number) {
-		state->ipi_number = xive_native_alloc_irq();
+		state->ipi_number = xive_native_alloc_irq(OPAL_XIVE_ANY_CHIP);
 		if (state->ipi_number == 0) {
 			pr_err("Failed to allocate IRQ !\n");
 			rc = -ENXIO;
diff --git a/arch/powerpc/platforms/powernv/ocxl.c b/arch/powerpc/platforms/powernv/ocxl.c
index 8c65aac..fb8f99a 100644
--- a/arch/powerpc/platforms/powernv/ocxl.c
+++ b/arch/powerpc/platforms/powernv/ocxl.c
@@ -487,24 +487,8 @@ int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle)
 
 int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr)
 {
-	__be64 flags, trigger_page;
-	s64 rc;
-	u32 hwirq;
-
-	hwirq = xive_native_alloc_irq();
-	if (!hwirq)
-		return -ENOENT;
-
-	rc = opal_xive_get_irq_info(hwirq, &flags, NULL, &trigger_page, NULL,
-				NULL);
-	if (rc || !trigger_page) {
-		xive_native_free_irq(hwirq);
-		return -ENOENT;
-	}
-	*irq = hwirq;
-	*trigger_addr = be64_to_cpu(trigger_page);
-	return 0;
-
+	return xive_native_alloc_get_irq_info(OPAL_XIVE_ANY_CHIP, irq,
+						trigger_addr);
 }
 EXPORT_SYMBOL_GPL(pnv_ocxl_alloc_xive_irq);
 
diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
index 0ff6b73..c450838 100644
--- a/arch/powerpc/sysdev/xive/native.c
+++ b/arch/powerpc/sysdev/xive/native.c
@@ -279,12 +279,12 @@ static int xive_native_get_ipi(unsigned int cpu, struct xive_cpu *xc)
 }
 #endif /* CONFIG_SMP */
 
-u32 xive_native_alloc_irq(void)
+u32 xive_native_alloc_irq(u32 chip_id)
 {
 	s64 rc;
 
 	for (;;) {
-		rc = opal_xive_allocate_irq(OPAL_XIVE_ANY_CHIP);
+		rc = opal_xive_allocate_irq(chip_id);
 		if (rc != OPAL_BUSY)
 			break;
 		msleep(OPAL_BUSY_DELAY_MS);
@@ -295,6 +295,29 @@ u32 xive_native_alloc_irq(void)
 }
 EXPORT_SYMBOL_GPL(xive_native_alloc_irq);
 
+int xive_native_alloc_get_irq_info(u32 chip_id, u32 *irq, u64 *trigger_addr)
+{
+	__be64 flags, trigger_page;
+	u32 hwirq;
+	s64 rc;
+
+	hwirq = xive_native_alloc_irq(chip_id);
+	if (!hwirq)
+		return -ENOENT;
+
+	rc = opal_xive_get_irq_info(hwirq, &flags, NULL, &trigger_page, NULL,
+				NULL);
+	if (rc || !trigger_page) {
+		xive_native_free_irq(hwirq);
+		return -ENOENT;
+	}
+	*irq = hwirq;
+	*trigger_addr = be64_to_cpu(trigger_page);
+
+	return 0;
+}
+EXPORT_SYMBOL(xive_native_alloc_get_irq_info);
+
 void xive_native_free_irq(u32 irq)
 {
 	for (;;) {










[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux