Re: [PATCH v2 04/16] KVM: PPC: Book3S HV: XIVE: add a control to initialize a source
On Tue, Mar 12, 2019 at 04:19:35PM +0100, Cédric Le Goater wrote:
> On 2/25/19 3:10 AM, David Gibson wrote:
> > On Fri, Feb 22, 2019 at 12:28:28PM +0100, Cédric Le Goater wrote:
> >> The associated HW interrupt source is simply allocated at the OPAL/HW
> >> level and then MASKED. KVM only needs to know about its type: LSI or
> >> MSI.
> >>
> >> Signed-off-by: Cédric Le Goater
> >> ---
> >> arch/powerpc/include/uapi/asm/kvm.h| 5 +
> >> arch/powerpc/kvm/book3s_xive.h | 10 ++
> >> arch/powerpc/kvm/book3s_xive.c | 8 +-
> >> arch/powerpc/kvm/book3s_xive_native.c | 114 +
> >> Documentation/virtual/kvm/devices/xive.txt | 15 +++
> >> 5 files changed, 148 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/arch/powerpc/include/uapi/asm/kvm.h
> >> b/arch/powerpc/include/uapi/asm/kvm.h
> >> index b002c0c67787..a9ad99f2a11b 100644
> >> --- a/arch/powerpc/include/uapi/asm/kvm.h
> >> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> >> @@ -677,5 +677,10 @@ struct kvm_ppc_cpu_char {
> >>
> >> /* POWER9 XIVE Native Interrupt Controller */
> >> #define KVM_DEV_XIVE_GRP_CTRL 1
> >> +#define KVM_DEV_XIVE_GRP_SOURCE 2 /* 64-bit source
> >> attributes */
> >> +
> >> +/* Layout of 64-bit XIVE source attribute values */
> >> +#define KVM_XIVE_LEVEL_SENSITIVE (1ULL << 0)
> >> +#define KVM_XIVE_LEVEL_ASSERTED (1ULL << 1)
> >>
> >> #endif /* __LINUX_KVM_POWERPC_H */
> >> diff --git a/arch/powerpc/kvm/book3s_xive.h
> >> b/arch/powerpc/kvm/book3s_xive.h
> >> index bcb1bbcf0359..f22f2d46d0f0 100644
> >> --- a/arch/powerpc/kvm/book3s_xive.h
> >> +++ b/arch/powerpc/kvm/book3s_xive.h
> >> @@ -12,6 +12,13 @@
> >> #ifdef CONFIG_KVM_XICS
> >> #include "book3s_xics.h"
> >>
> >> +/*
> >> + * The XIVE IRQ number space is aligned with the XICS IRQ number
> >> + * space, CPU IPIs being allocated in the first 4K.
> >
> > We do align these in qemu, but I don't see that the kernel part
> > cares: as far as it's concerned only one of XICS or XIVE is active at
> > a time, and the irq numbers are chosen by userspace.
>
> There is some relation with userspace nevertheless. The KVM device does
> not remap the numbers to some other range today and the limits are fixed
> values. Checks are being done in the has_attr() and the set_attr().
Hrm. I still think the comment needs to describe what the constraint
is from the point of view of the kernel, which this isn't. Maybe, "we
allow irqs in the range X..Y (allowing userspace to do ...)"
>
> >> + */
> >> +#define KVMPPC_XIVE_FIRST_IRQ 0
> >> +#define KVMPPC_XIVE_NR_IRQS KVMPPC_XICS_NR_IRQS
> >> +
> >> /*
> >> * State for one guest irq source.
> >> *
> >> @@ -253,6 +260,9 @@ extern int (*__xive_vm_h_eoi)(struct kvm_vcpu *vcpu,
> >> unsigned long xirr);
> >> */
> >> void kvmppc_xive_disable_vcpu_interrupts(struct kvm_vcpu *vcpu);
> >> int kvmppc_xive_debug_show_queues(struct seq_file *m, struct kvm_vcpu
> >> *vcpu);
> >> +struct kvmppc_xive_src_block *kvmppc_xive_create_src_block(
> >> + struct kvmppc_xive *xive, int irq);
> >> +void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb);
> >>
> >> #endif /* CONFIG_KVM_XICS */
> >> #endif /* _KVM_PPC_BOOK3S_XICS_H */
> >> diff --git a/arch/powerpc/kvm/book3s_xive.c
> >> b/arch/powerpc/kvm/book3s_xive.c
> >> index d1cc18a5b1c4..6f950ecb3592 100644
> >> --- a/arch/powerpc/kvm/book3s_xive.c
> >> +++ b/arch/powerpc/kvm/book3s_xive.c
> >
> > I wonder if we should rename this book3s_xics_on_xive.c or something
> > at some point, I keep getting confused because I forget that this is
> > only dealing with host xive, not guest xive.
>
> I am fine with renaming. Any objections ? book3s_xics_p9.c ?
Hm, maybe?
> >> @@ -1485,8 +1485,8 @@ static int xive_get_source(struct kvmppc_xive *xive,
> >> long irq, u64 addr)
> >>return 0;
> >> }
> >>
> >> -static struct kvmppc_xive_src_block *xive_create_src_block(struct
> >> kvmppc_xive *xive,
> >> - int irq)
> >> +struct kvmppc_xive_src_block *kvmppc_xive_create_src_block(
> >> + struct kvmppc_xive *xive, int irq)
> >> {
> >>struct kvm *kvm = xive->kvm;
> >>struct kvmppc_xive_src_block *sb;
> >
> > It's odd that this function, now used from the xive-on-xive path as
> > well as the xics-on-xive path references KVMPPC_XICS_ICS_SHIFT a few
> > lines down from this change.
>
> Yes. This is because of the definition of the struct kvmppc_xive_src_block.
>
> We could introduce new defines for XIVE or a common set of defines for
> XICS and XIVE.
I think making common definitions would be best, if possible.
>
> >> @@ -1565,7 +1565,7 @@ static int xive_set_source(struct kvmppc_xive *xive,
> >> long irq, u64 addr)
> >>sb = kvmppc_xive_find_source(xive, irq, &idx);
> >>if (!sb) {
> >>pr_devel("No source, creating source block...\n");
> >> - sb = xive_create_src_block(xive, irq);
>
Re: [PATCH v2 04/16] KVM: PPC: Book3S HV: XIVE: add a control to initialize a source
On 2/25/19 3:10 AM, David Gibson wrote:
> On Fri, Feb 22, 2019 at 12:28:28PM +0100, Cédric Le Goater wrote:
>> The associated HW interrupt source is simply allocated at the OPAL/HW
>> level and then MASKED. KVM only needs to know about its type: LSI or
>> MSI.
>>
>> Signed-off-by: Cédric Le Goater
>> ---
>> arch/powerpc/include/uapi/asm/kvm.h| 5 +
>> arch/powerpc/kvm/book3s_xive.h | 10 ++
>> arch/powerpc/kvm/book3s_xive.c | 8 +-
>> arch/powerpc/kvm/book3s_xive_native.c | 114 +
>> Documentation/virtual/kvm/devices/xive.txt | 15 +++
>> 5 files changed, 148 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h
>> b/arch/powerpc/include/uapi/asm/kvm.h
>> index b002c0c67787..a9ad99f2a11b 100644
>> --- a/arch/powerpc/include/uapi/asm/kvm.h
>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
>> @@ -677,5 +677,10 @@ struct kvm_ppc_cpu_char {
>>
>> /* POWER9 XIVE Native Interrupt Controller */
>> #define KVM_DEV_XIVE_GRP_CTRL 1
>> +#define KVM_DEV_XIVE_GRP_SOURCE 2 /* 64-bit source
>> attributes */
>> +
>> +/* Layout of 64-bit XIVE source attribute values */
>> +#define KVM_XIVE_LEVEL_SENSITIVE(1ULL << 0)
>> +#define KVM_XIVE_LEVEL_ASSERTED (1ULL << 1)
>>
>> #endif /* __LINUX_KVM_POWERPC_H */
>> diff --git a/arch/powerpc/kvm/book3s_xive.h b/arch/powerpc/kvm/book3s_xive.h
>> index bcb1bbcf0359..f22f2d46d0f0 100644
>> --- a/arch/powerpc/kvm/book3s_xive.h
>> +++ b/arch/powerpc/kvm/book3s_xive.h
>> @@ -12,6 +12,13 @@
>> #ifdef CONFIG_KVM_XICS
>> #include "book3s_xics.h"
>>
>> +/*
>> + * The XIVE IRQ number space is aligned with the XICS IRQ number
>> + * space, CPU IPIs being allocated in the first 4K.
>
> We do align these in qemu, but I don't see that the kernel part
> cares: as far as it's concerned only one of XICS or XIVE is active at
> a time, and the irq numbers are chosen by userspace.
There is some relation with userspace nevertheless. The KVM device does
not remap the numbers to some other range today and the limits are fixed
values. Checks are being done in the has_attr() and the set_attr().
>> + */
>> +#define KVMPPC_XIVE_FIRST_IRQ 0
>> +#define KVMPPC_XIVE_NR_IRQS KVMPPC_XICS_NR_IRQS
>> +
>> /*
>> * State for one guest irq source.
>> *
>> @@ -253,6 +260,9 @@ extern int (*__xive_vm_h_eoi)(struct kvm_vcpu *vcpu,
>> unsigned long xirr);
>> */
>> void kvmppc_xive_disable_vcpu_interrupts(struct kvm_vcpu *vcpu);
>> int kvmppc_xive_debug_show_queues(struct seq_file *m, struct kvm_vcpu
>> *vcpu);
>> +struct kvmppc_xive_src_block *kvmppc_xive_create_src_block(
>> +struct kvmppc_xive *xive, int irq);
>> +void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb);
>>
>> #endif /* CONFIG_KVM_XICS */
>> #endif /* _KVM_PPC_BOOK3S_XICS_H */
>> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
>> index d1cc18a5b1c4..6f950ecb3592 100644
>> --- a/arch/powerpc/kvm/book3s_xive.c
>> +++ b/arch/powerpc/kvm/book3s_xive.c
>
> I wonder if we should rename this book3s_xics_on_xive.c or something
> at some point, I keep getting confused because I forget that this is
> only dealing with host xive, not guest xive.
I am fine with renaming. Any objections ? book3s_xics_p9.c ?
>> @@ -1485,8 +1485,8 @@ static int xive_get_source(struct kvmppc_xive *xive,
>> long irq, u64 addr)
>> return 0;
>> }
>>
>> -static struct kvmppc_xive_src_block *xive_create_src_block(struct
>> kvmppc_xive *xive,
>> - int irq)
>> +struct kvmppc_xive_src_block *kvmppc_xive_create_src_block(
>> +struct kvmppc_xive *xive, int irq)
>> {
>> struct kvm *kvm = xive->kvm;
>> struct kvmppc_xive_src_block *sb;
>
> It's odd that this function, now used from the xive-on-xive path as
> well as the xics-on-xive path references KVMPPC_XICS_ICS_SHIFT a few
> lines down from this change.
Yes. This is because of the definition of the struct kvmppc_xive_src_block.
We could introduce new defines for XIVE or a common set of defines for
XICS and XIVE.
>> @@ -1565,7 +1565,7 @@ static int xive_set_source(struct kvmppc_xive *xive,
>> long irq, u64 addr)
>> sb = kvmppc_xive_find_source(xive, irq, &idx);
>> if (!sb) {
>> pr_devel("No source, creating source block...\n");
>> -sb = xive_create_src_block(xive, irq);
>> +sb = kvmppc_xive_create_src_block(xive, irq);
>> if (!sb) {
>> pr_devel("Failed to create block...\n");
>> return -ENOMEM;
>> @@ -1789,7 +1789,7 @@ static void kvmppc_xive_cleanup_irq(u32 hw_num, struct
>> xive_irq_data *xd)
>> xive_cleanup_irq_data(xd);
>> }
>>
>> -static void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb)
>> +void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb)
>> {
>> int i;
>>
>> diff --git a/arch/powerpc/kvm/book3s_xi
Re: [PATCH v2 04/16] KVM: PPC: Book3S HV: XIVE: add a control to initialize a source
On Tue, Feb 26, 2019 at 03:25:15PM +1100, Paul Mackerras wrote:
> On Mon, Feb 25, 2019 at 01:10:12PM +1100, David Gibson wrote:
> > On Fri, Feb 22, 2019 at 12:28:28PM +0100, Cédric Le Goater wrote:
> > > + /*
> > > + * If the source doesn't already have an IPI, allocate
> > > + * one and get the corresponding data
> > > + */
> > > + if (!state->ipi_number) {
> > > + state->ipi_number = xive_native_alloc_irq();
> > > + if (state->ipi_number == 0) {
> > > + pr_err("Failed to allocate IRQ !\n");
> > > + return -ENXIO;
> > > + }
> > > + xive_native_populate_irq_data(state->ipi_number,
> > > + &state->ipi_data);
> > > + pr_debug("%s allocated hw_irq=0x%x for irq=0x%lx\n", __func__,
> > > + state->ipi_number, irq);
> > > + }
> > > +
> > > + arch_spin_lock(&sb->lock);
> >
> > Why the direct call to arch_spin_lock() rather than just spin_lock()?
>
> He's sharing data structures with the xics-on-xive code, and that code
> has a real-mode variant, and in real mode we don't want to risk
> invoking lockdep code. Hence sb->lock is an arch_spinlock_t, and he
> has to use arch_spin_lock() on it.
Ah, right.
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH v2 04/16] KVM: PPC: Book3S HV: XIVE: add a control to initialize a source
On Mon, Feb 25, 2019 at 01:10:12PM +1100, David Gibson wrote:
> On Fri, Feb 22, 2019 at 12:28:28PM +0100, Cédric Le Goater wrote:
> > + /*
> > +* If the source doesn't already have an IPI, allocate
> > +* one and get the corresponding data
> > +*/
> > + if (!state->ipi_number) {
> > + state->ipi_number = xive_native_alloc_irq();
> > + if (state->ipi_number == 0) {
> > + pr_err("Failed to allocate IRQ !\n");
> > + return -ENXIO;
> > + }
> > + xive_native_populate_irq_data(state->ipi_number,
> > + &state->ipi_data);
> > + pr_debug("%s allocated hw_irq=0x%x for irq=0x%lx\n", __func__,
> > +state->ipi_number, irq);
> > + }
> > +
> > + arch_spin_lock(&sb->lock);
>
> Why the direct call to arch_spin_lock() rather than just spin_lock()?
He's sharing data structures with the xics-on-xive code, and that code
has a real-mode variant, and in real mode we don't want to risk
invoking lockdep code. Hence sb->lock is an arch_spinlock_t, and he
has to use arch_spin_lock() on it.
Paul.
Re: [PATCH v2 04/16] KVM: PPC: Book3S HV: XIVE: add a control to initialize a source
On Fri, Feb 22, 2019 at 12:28:28PM +0100, Cédric Le Goater wrote:
> The associated HW interrupt source is simply allocated at the OPAL/HW
> level and then MASKED. KVM only needs to know about its type: LSI or
> MSI.
I think it would be helpful to explain to the reader here that with
XIVE, all interrupts have a hardware source, even IPIs and virtual
device interrupts, for which we allocate a software-triggerable
interrupt source in the XIVE hardware.
>
> Signed-off-by: Cédric Le Goater
> ---
> arch/powerpc/include/uapi/asm/kvm.h| 5 +
> arch/powerpc/kvm/book3s_xive.h | 10 ++
> arch/powerpc/kvm/book3s_xive.c | 8 +-
> arch/powerpc/kvm/book3s_xive_native.c | 114 +
> Documentation/virtual/kvm/devices/xive.txt | 15 +++
> 5 files changed, 148 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/uapi/asm/kvm.h
> b/arch/powerpc/include/uapi/asm/kvm.h
> index b002c0c67787..a9ad99f2a11b 100644
> --- a/arch/powerpc/include/uapi/asm/kvm.h
> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> @@ -677,5 +677,10 @@ struct kvm_ppc_cpu_char {
>
> /* POWER9 XIVE Native Interrupt Controller */
> #define KVM_DEV_XIVE_GRP_CTRL1
> +#define KVM_DEV_XIVE_GRP_SOURCE 2 /* 64-bit source
> attributes */
> +
> +/* Layout of 64-bit XIVE source attribute values */
> +#define KVM_XIVE_LEVEL_SENSITIVE (1ULL << 0)
> +#define KVM_XIVE_LEVEL_ASSERTED (1ULL << 1)
>
> #endif /* __LINUX_KVM_POWERPC_H */
> diff --git a/arch/powerpc/kvm/book3s_xive.h b/arch/powerpc/kvm/book3s_xive.h
> index bcb1bbcf0359..f22f2d46d0f0 100644
> --- a/arch/powerpc/kvm/book3s_xive.h
> +++ b/arch/powerpc/kvm/book3s_xive.h
> @@ -12,6 +12,13 @@
> #ifdef CONFIG_KVM_XICS
> #include "book3s_xics.h"
>
> +/*
> + * The XIVE IRQ number space is aligned with the XICS IRQ number
> + * space, CPU IPIs being allocated in the first 4K.
> + */
> +#define KVMPPC_XIVE_FIRST_IRQ0
> +#define KVMPPC_XIVE_NR_IRQS KVMPPC_XICS_NR_IRQS
> +
> /*
> * State for one guest irq source.
> *
> @@ -253,6 +260,9 @@ extern int (*__xive_vm_h_eoi)(struct kvm_vcpu *vcpu,
> unsigned long xirr);
> */
> void kvmppc_xive_disable_vcpu_interrupts(struct kvm_vcpu *vcpu);
> int kvmppc_xive_debug_show_queues(struct seq_file *m, struct kvm_vcpu *vcpu);
> +struct kvmppc_xive_src_block *kvmppc_xive_create_src_block(
> + struct kvmppc_xive *xive, int irq);
> +void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb);
So we're using the same source block data structure (effectively a
2-level tree) that the XICS-on-XIVE code uses. That would be worth
mentioning as a design choice in the patch description.
>
> #endif /* CONFIG_KVM_XICS */
> #endif /* _KVM_PPC_BOOK3S_XICS_H */
> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> index d1cc18a5b1c4..6f950ecb3592 100644
> --- a/arch/powerpc/kvm/book3s_xive.c
> +++ b/arch/powerpc/kvm/book3s_xive.c
> @@ -1485,8 +1485,8 @@ static int xive_get_source(struct kvmppc_xive *xive,
> long irq, u64 addr)
> return 0;
> }
>
> -static struct kvmppc_xive_src_block *xive_create_src_block(struct
> kvmppc_xive *xive,
> -int irq)
> +struct kvmppc_xive_src_block *kvmppc_xive_create_src_block(
> + struct kvmppc_xive *xive, int irq)
> {
> struct kvm *kvm = xive->kvm;
> struct kvmppc_xive_src_block *sb;
> @@ -1565,7 +1565,7 @@ static int xive_set_source(struct kvmppc_xive *xive,
> long irq, u64 addr)
> sb = kvmppc_xive_find_source(xive, irq, &idx);
> if (!sb) {
> pr_devel("No source, creating source block...\n");
> - sb = xive_create_src_block(xive, irq);
> + sb = kvmppc_xive_create_src_block(xive, irq);
> if (!sb) {
> pr_devel("Failed to create block...\n");
> return -ENOMEM;
> @@ -1789,7 +1789,7 @@ static void kvmppc_xive_cleanup_irq(u32 hw_num, struct
> xive_irq_data *xd)
> xive_cleanup_irq_data(xd);
> }
>
> -static void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb)
> +void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb)
> {
> int i;
>
> diff --git a/arch/powerpc/kvm/book3s_xive_native.c
> b/arch/powerpc/kvm/book3s_xive_native.c
> index 1f3da47a4a6a..a9b2d2d9af99 100644
> --- a/arch/powerpc/kvm/book3s_xive_native.c
> +++ b/arch/powerpc/kvm/book3s_xive_native.c
> @@ -31,6 +31,29 @@
>
> #include "book3s_xive.h"
>
> +/*
> + * TODO: introduce a common template file with the XIVE native layer
> + * and the XICS-on-XIVE glue for the utility functions
> + */
> +#define __x_eoi_page(xd) ((void __iomem *)((xd)->eoi_mmio))
> +#define __x_trig_page(xd)((void __iomem *)((xd)->trig_mmio))
> +#define __x_readq__raw_readq
> +#define __x_writeq __raw_writeq
> +
> +static u8 xive_vm_esb_load(struct xive_irq_data *xd, u32 offset)
> +{
> + u64 val;
> +
Re: [PATCH v2 04/16] KVM: PPC: Book3S HV: XIVE: add a control to initialize a source
On Fri, Feb 22, 2019 at 12:28:28PM +0100, Cédric Le Goater wrote:
> The associated HW interrupt source is simply allocated at the OPAL/HW
> level and then MASKED. KVM only needs to know about its type: LSI or
> MSI.
>
> Signed-off-by: Cédric Le Goater
> ---
> arch/powerpc/include/uapi/asm/kvm.h| 5 +
> arch/powerpc/kvm/book3s_xive.h | 10 ++
> arch/powerpc/kvm/book3s_xive.c | 8 +-
> arch/powerpc/kvm/book3s_xive_native.c | 114 +
> Documentation/virtual/kvm/devices/xive.txt | 15 +++
> 5 files changed, 148 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/uapi/asm/kvm.h
> b/arch/powerpc/include/uapi/asm/kvm.h
> index b002c0c67787..a9ad99f2a11b 100644
> --- a/arch/powerpc/include/uapi/asm/kvm.h
> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> @@ -677,5 +677,10 @@ struct kvm_ppc_cpu_char {
>
> /* POWER9 XIVE Native Interrupt Controller */
> #define KVM_DEV_XIVE_GRP_CTRL1
> +#define KVM_DEV_XIVE_GRP_SOURCE 2 /* 64-bit source
> attributes */
> +
> +/* Layout of 64-bit XIVE source attribute values */
> +#define KVM_XIVE_LEVEL_SENSITIVE (1ULL << 0)
> +#define KVM_XIVE_LEVEL_ASSERTED (1ULL << 1)
>
> #endif /* __LINUX_KVM_POWERPC_H */
> diff --git a/arch/powerpc/kvm/book3s_xive.h b/arch/powerpc/kvm/book3s_xive.h
> index bcb1bbcf0359..f22f2d46d0f0 100644
> --- a/arch/powerpc/kvm/book3s_xive.h
> +++ b/arch/powerpc/kvm/book3s_xive.h
> @@ -12,6 +12,13 @@
> #ifdef CONFIG_KVM_XICS
> #include "book3s_xics.h"
>
> +/*
> + * The XIVE IRQ number space is aligned with the XICS IRQ number
> + * space, CPU IPIs being allocated in the first 4K.
We do align these in qemu, but I don't see that the kernel part
cares: as far as it's concerned only one of XICS or XIVE is active at
a time, and the irq numbers are chosen by userspace.
> + */
> +#define KVMPPC_XIVE_FIRST_IRQ0
> +#define KVMPPC_XIVE_NR_IRQS KVMPPC_XICS_NR_IRQS
> +
> /*
> * State for one guest irq source.
> *
> @@ -253,6 +260,9 @@ extern int (*__xive_vm_h_eoi)(struct kvm_vcpu *vcpu,
> unsigned long xirr);
> */
> void kvmppc_xive_disable_vcpu_interrupts(struct kvm_vcpu *vcpu);
> int kvmppc_xive_debug_show_queues(struct seq_file *m, struct kvm_vcpu *vcpu);
> +struct kvmppc_xive_src_block *kvmppc_xive_create_src_block(
> + struct kvmppc_xive *xive, int irq);
> +void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb);
>
> #endif /* CONFIG_KVM_XICS */
> #endif /* _KVM_PPC_BOOK3S_XICS_H */
> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> index d1cc18a5b1c4..6f950ecb3592 100644
> --- a/arch/powerpc/kvm/book3s_xive.c
> +++ b/arch/powerpc/kvm/book3s_xive.c
I wonder if we should rename this book3s_xics_on_xive.c or something
at some point, I keep getting confused because I forget that this is
only dealing with host xive, not guest xive.
> @@ -1485,8 +1485,8 @@ static int xive_get_source(struct kvmppc_xive *xive,
> long irq, u64 addr)
> return 0;
> }
>
> -static struct kvmppc_xive_src_block *xive_create_src_block(struct
> kvmppc_xive *xive,
> -int irq)
> +struct kvmppc_xive_src_block *kvmppc_xive_create_src_block(
> + struct kvmppc_xive *xive, int irq)
> {
> struct kvm *kvm = xive->kvm;
> struct kvmppc_xive_src_block *sb;
It's odd that this function, now used from the xive-on-xive path as
well as the xics-on-xive path references KVMPPC_XICS_ICS_SHIFT a few
lines down from this change.
> @@ -1565,7 +1565,7 @@ static int xive_set_source(struct kvmppc_xive *xive,
> long irq, u64 addr)
> sb = kvmppc_xive_find_source(xive, irq, &idx);
> if (!sb) {
> pr_devel("No source, creating source block...\n");
> - sb = xive_create_src_block(xive, irq);
> + sb = kvmppc_xive_create_src_block(xive, irq);
> if (!sb) {
> pr_devel("Failed to create block...\n");
> return -ENOMEM;
> @@ -1789,7 +1789,7 @@ static void kvmppc_xive_cleanup_irq(u32 hw_num, struct
> xive_irq_data *xd)
> xive_cleanup_irq_data(xd);
> }
>
> -static void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb)
> +void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb)
> {
> int i;
>
> diff --git a/arch/powerpc/kvm/book3s_xive_native.c
> b/arch/powerpc/kvm/book3s_xive_native.c
> index 1f3da47a4a6a..a9b2d2d9af99 100644
> --- a/arch/powerpc/kvm/book3s_xive_native.c
> +++ b/arch/powerpc/kvm/book3s_xive_native.c
> @@ -31,6 +31,29 @@
>
> #include "book3s_xive.h"
>
> +/*
> + * TODO: introduce a common template file with the XIVE native layer
> + * and the XICS-on-XIVE glue for the utility functions
> + */
> +#define __x_eoi_page(xd) ((void __iomem *)((xd)->eoi_mmio))
> +#define __x_trig_page(xd)((void __iomem *)((xd)->trig_mmio))
> +#define __x_readq__raw_readq
> +#define __x_writeq __
[PATCH v2 04/16] KVM: PPC: Book3S HV: XIVE: add a control to initialize a source
The associated HW interrupt source is simply allocated at the OPAL/HW
level and then MASKED. KVM only needs to know about its type: LSI or
MSI.
Signed-off-by: Cédric Le Goater
---
arch/powerpc/include/uapi/asm/kvm.h| 5 +
arch/powerpc/kvm/book3s_xive.h | 10 ++
arch/powerpc/kvm/book3s_xive.c | 8 +-
arch/powerpc/kvm/book3s_xive_native.c | 114 +
Documentation/virtual/kvm/devices/xive.txt | 15 +++
5 files changed, 148 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/uapi/asm/kvm.h
b/arch/powerpc/include/uapi/asm/kvm.h
index b002c0c67787..a9ad99f2a11b 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -677,5 +677,10 @@ struct kvm_ppc_cpu_char {
/* POWER9 XIVE Native Interrupt Controller */
#define KVM_DEV_XIVE_GRP_CTRL 1
+#define KVM_DEV_XIVE_GRP_SOURCE2 /* 64-bit source
attributes */
+
+/* Layout of 64-bit XIVE source attribute values */
+#define KVM_XIVE_LEVEL_SENSITIVE (1ULL << 0)
+#define KVM_XIVE_LEVEL_ASSERTED(1ULL << 1)
#endif /* __LINUX_KVM_POWERPC_H */
diff --git a/arch/powerpc/kvm/book3s_xive.h b/arch/powerpc/kvm/book3s_xive.h
index bcb1bbcf0359..f22f2d46d0f0 100644
--- a/arch/powerpc/kvm/book3s_xive.h
+++ b/arch/powerpc/kvm/book3s_xive.h
@@ -12,6 +12,13 @@
#ifdef CONFIG_KVM_XICS
#include "book3s_xics.h"
+/*
+ * The XIVE IRQ number space is aligned with the XICS IRQ number
+ * space, CPU IPIs being allocated in the first 4K.
+ */
+#define KVMPPC_XIVE_FIRST_IRQ 0
+#define KVMPPC_XIVE_NR_IRQSKVMPPC_XICS_NR_IRQS
+
/*
* State for one guest irq source.
*
@@ -253,6 +260,9 @@ extern int (*__xive_vm_h_eoi)(struct kvm_vcpu *vcpu,
unsigned long xirr);
*/
void kvmppc_xive_disable_vcpu_interrupts(struct kvm_vcpu *vcpu);
int kvmppc_xive_debug_show_queues(struct seq_file *m, struct kvm_vcpu *vcpu);
+struct kvmppc_xive_src_block *kvmppc_xive_create_src_block(
+ struct kvmppc_xive *xive, int irq);
+void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb);
#endif /* CONFIG_KVM_XICS */
#endif /* _KVM_PPC_BOOK3S_XICS_H */
diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index d1cc18a5b1c4..6f950ecb3592 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -1485,8 +1485,8 @@ static int xive_get_source(struct kvmppc_xive *xive, long
irq, u64 addr)
return 0;
}
-static struct kvmppc_xive_src_block *xive_create_src_block(struct kvmppc_xive
*xive,
- int irq)
+struct kvmppc_xive_src_block *kvmppc_xive_create_src_block(
+ struct kvmppc_xive *xive, int irq)
{
struct kvm *kvm = xive->kvm;
struct kvmppc_xive_src_block *sb;
@@ -1565,7 +1565,7 @@ static int xive_set_source(struct kvmppc_xive *xive, long
irq, u64 addr)
sb = kvmppc_xive_find_source(xive, irq, &idx);
if (!sb) {
pr_devel("No source, creating source block...\n");
- sb = xive_create_src_block(xive, irq);
+ sb = kvmppc_xive_create_src_block(xive, irq);
if (!sb) {
pr_devel("Failed to create block...\n");
return -ENOMEM;
@@ -1789,7 +1789,7 @@ static void kvmppc_xive_cleanup_irq(u32 hw_num, struct
xive_irq_data *xd)
xive_cleanup_irq_data(xd);
}
-static void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb)
+void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb)
{
int i;
diff --git a/arch/powerpc/kvm/book3s_xive_native.c
b/arch/powerpc/kvm/book3s_xive_native.c
index 1f3da47a4a6a..a9b2d2d9af99 100644
--- a/arch/powerpc/kvm/book3s_xive_native.c
+++ b/arch/powerpc/kvm/book3s_xive_native.c
@@ -31,6 +31,29 @@
#include "book3s_xive.h"
+/*
+ * TODO: introduce a common template file with the XIVE native layer
+ * and the XICS-on-XIVE glue for the utility functions
+ */
+#define __x_eoi_page(xd) ((void __iomem *)((xd)->eoi_mmio))
+#define __x_trig_page(xd) ((void __iomem *)((xd)->trig_mmio))
+#define __x_readq __raw_readq
+#define __x_writeq __raw_writeq
+
+static u8 xive_vm_esb_load(struct xive_irq_data *xd, u32 offset)
+{
+ u64 val;
+
+ if (xd->flags & XIVE_IRQ_FLAG_SHIFT_BUG)
+ offset |= offset << 4;
+
+ val = __x_readq(__x_eoi_page(xd) + offset);
+#ifdef __LITTLE_ENDIAN__
+ val >>= 64-8;
+#endif
+ return (u8)val;
+}
+
static void kvmppc_xive_native_cleanup_queue(struct kvm_vcpu *vcpu, int prio)
{
struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
@@ -153,12 +176,89 @@ int kvmppc_xive_native_connect_vcpu(struct kvm_device
*dev,
return rc;
}
+static int kvmppc_xive_native_set_source(struct kvmppc_xive *xive, long irq,
+u64 addr)
+{
+ struct kvmppc_xive_src_block *sb;
+ struct kvmppc_xive_irq_state *state;
+
