Re: [Qemu-devel] [RFC PATCH v2 14/21] ppc/xive: add support for the SET_OS_PENDING command

2017-09-28 Thread Benjamin Herrenschmidt 
On Wed, 2017-09-20 at 11:47 +0200, Cédric Le Goater wrote:
> > > @@ -162,7 +162,14 @@ static bool spapr_xive_tm_is_readonly(uint8_t index)
> > >   static void spapr_xive_tm_write_special(ICPState *icp, hwaddr offset,
> > > uint64_t value, unsigned size)
> > >   {
> > > -/* TODO: support TM_SPC_SET_OS_PENDING */
> > > +if (offset == TM_SPC_SET_OS_PENDING && size == 1) {
> > > +icp->tima_os[TM_IPB] |= priority_to_ipb(value & 0xff);
> > > +icp->tima_os[TM_PIPR] = ipb_to_pipr(icp->tima_os[TM_IPB]);
> > 
> > This only lets the cpu raise bits in the IPB, never clear them.> Is that 
> > right?  
> 
> The clear is done when the OS acks the interrupt.
> 
> > I don't see how you'd implement the handling of multiple
> > priorities without being able to clear bits here.
> 
> I am not sure how this command should be used from the OS. 
> Currently, I only see KVM handling it in the XICS/XIVE glue.
> I need to take a closer look.

It's a way to avoid the SW replay on EOI.

IE, assume you have 2 interrupts in the queue. You take the exception,
ack the first one, process it etc...

Then you EOI, the HW won't send a second notification. You need to look
at the queue and continue consuming until it's empty.

Today Linux checks the queue on EOI and use a SW mechanism to
synthetize a new pseudo-external interrupt.

This MMIO command would allow the OS to instead set back the
corresponding priority bit to 1 in the IPB and cause the HW to re-emit
the interrupt instead of SW.

Linux doesn't use this today because DD1 didn't support it for the HV
level, but other OSes might and we also might use it when we do groups,
thus allowing redistribution.

Cheers,
Ben.

Re: [Qemu-devel] [RFC PATCH v2 14/21] ppc/xive: add support for the SET_OS_PENDING command

2017-09-20 Thread Cédric Le Goater 
On 09/19/2017 09:55 AM, David Gibson wrote:
> On Mon, Sep 11, 2017 at 07:12:28PM +0200, Cédric Le Goater wrote:
>> Adjusting the Interrupt Pending Buffer for the O/S would allow a CPU
>> to process event queues of other priorities during one physical
>> interrupt cycle. This is not currently used by the XIVE support for
>> sPAPR in Linux but it is by the hypervisor.
>>
>> Signed-off-by: Cédric Le Goater 
>> ---
>>  hw/intc/spapr_xive.c | 9 -
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
>> index ad3ff91b13ea..ad3f03e37401 100644
>> --- a/hw/intc/spapr_xive.c
>> +++ b/hw/intc/spapr_xive.c
>> @@ -162,7 +162,14 @@ static bool spapr_xive_tm_is_readonly(uint8_t index)
>>  static void spapr_xive_tm_write_special(ICPState *icp, hwaddr offset,
>>uint64_t value, unsigned size)
>>  {
>> -/* TODO: support TM_SPC_SET_OS_PENDING */
>> +if (offset == TM_SPC_SET_OS_PENDING && size == 1) {
>> +icp->tima_os[TM_IPB] |= priority_to_ipb(value & 0xff);
>> +icp->tima_os[TM_PIPR] = ipb_to_pipr(icp->tima_os[TM_IPB]);
> 
> This only lets the cpu raise bits in the IPB, never clear them.> Is that 
> right?  

The clear is done when the OS acks the interrupt.

> I don't see how you'd implement the handling of multiple
> priorities without being able to clear bits here.

I am not sure how this command should be used from the OS. 
Currently, I only see KVM handling it in the XICS/XIVE glue.
I need to take a closer look.

C.
 


>> +spapr_xive_icp_notify(icp);
>> +} else {
>> +qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid TIMA write @%"
>> +  HWADDR_PRIx" size %d\n", offset, size);
>> +}
>>  
>>  /* TODO: support TM_SPC_ACK_OS_EL */
>>  }
>

Re: [Qemu-devel] [RFC PATCH v2 14/21] ppc/xive: add support for the SET_OS_PENDING command

2017-09-19 Thread David Gibson 
On Mon, Sep 11, 2017 at 07:12:28PM +0200, Cédric Le Goater wrote:
> Adjusting the Interrupt Pending Buffer for the O/S would allow a CPU
> to process event queues of other priorities during one physical
> interrupt cycle. This is not currently used by the XIVE support for
> sPAPR in Linux but it is by the hypervisor.
> 
> Signed-off-by: Cédric Le Goater 
> ---
>  hw/intc/spapr_xive.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index ad3ff91b13ea..ad3f03e37401 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -162,7 +162,14 @@ static bool spapr_xive_tm_is_readonly(uint8_t index)
>  static void spapr_xive_tm_write_special(ICPState *icp, hwaddr offset,
>uint64_t value, unsigned size)
>  {
> -/* TODO: support TM_SPC_SET_OS_PENDING */
> +if (offset == TM_SPC_SET_OS_PENDING && size == 1) {
> +icp->tima_os[TM_IPB] |= priority_to_ipb(value & 0xff);
> +icp->tima_os[TM_PIPR] = ipb_to_pipr(icp->tima_os[TM_IPB]);

This only lets the cpu raise bits in the IPB, never clear them.  Is
that right?  I don't see how you'd implement the handling of multiple
priorities without being able to clear bits here.

> +spapr_xive_icp_notify(icp);
> +} else {
> +qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid TIMA write @%"
> +  HWADDR_PRIx" size %d\n", offset, size);
> +}
>  
>  /* TODO: support TM_SPC_ACK_OS_EL */
>  }

-- 
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

[Qemu-devel] [RFC PATCH v2 14/21] ppc/xive: add support for the SET_OS_PENDING command

2017-09-11 Thread Cédric Le Goater 
Adjusting the Interrupt Pending Buffer for the O/S would allow a CPU
to process event queues of other priorities during one physical
interrupt cycle. This is not currently used by the XIVE support for
sPAPR in Linux but it is by the hypervisor.

Signed-off-by: Cédric Le Goater 
---
 hw/intc/spapr_xive.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index ad3ff91b13ea..ad3f03e37401 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -162,7 +162,14 @@ static bool spapr_xive_tm_is_readonly(uint8_t index)
 static void spapr_xive_tm_write_special(ICPState *icp, hwaddr offset,
   uint64_t value, unsigned size)
 {
-/* TODO: support TM_SPC_SET_OS_PENDING */
+if (offset == TM_SPC_SET_OS_PENDING && size == 1) {
+icp->tima_os[TM_IPB] |= priority_to_ipb(value & 0xff);
+icp->tima_os[TM_PIPR] = ipb_to_pipr(icp->tima_os[TM_IPB]);
+spapr_xive_icp_notify(icp);
+} else {
+qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid TIMA write @%"
+  HWADDR_PRIx" size %d\n", offset, size);
+}
 
 /* TODO: support TM_SPC_ACK_OS_EL */
 }
-- 
2.13.5