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 <ha...@linux.vnet.ibm.com> 
> > 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 (;;) {







Reply via email to