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 (;;) {
