* [PATCH] hw/intc: Handle software disabling of APIC correctly
@ 2022-07-12 14:18 Jay Khandkar
2022-07-29 16:45 ` Jay Khandkar
2022-07-29 17:09 ` Peter Maydell
0 siblings, 2 replies; 6+ messages in thread
From: Jay Khandkar @ 2022-07-12 14:18 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, mst, Jay Khandkar
When the local APIC is in a software disabled state, all local interrupt
sources must be masked and all attempts to unmask them should be
ignored. Currently, we don't do either. Fix this by handling it
correctly in apic_mem_write().
Signed-off-by: Jay Khandkar <jaykhandkar2002@gmail.com>
---
hw/intc/apic.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 3df11c34d6..493c70af62 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -792,9 +792,16 @@ static void apic_mem_write(void *opaque, hwaddr addr, uint64_t val,
s->dest_mode = val >> 28;
break;
case 0x0f:
- s->spurious_vec = val & 0x1ff;
- apic_update_irq(s);
- break;
+ {
+ s->spurious_vec = val & 0x1ff;
+ if (!(val & APIC_SPURIO_ENABLED)) {
+ for (int i = 0; i < APIC_LVT_NB; i++) {
+ s->lvt[i] |= APIC_LVT_MASKED;
+ }
+ }
+ apic_update_irq(s);
+ break;
+ }
case 0x10 ... 0x17:
case 0x18 ... 0x1f:
case 0x20 ... 0x27:
@@ -812,6 +819,9 @@ static void apic_mem_write(void *opaque, hwaddr addr, uint64_t val,
case 0x32 ... 0x37:
{
int n = index - 0x32;
+ if (!(s->spurious_vec & APIC_SPURIO_ENABLED)) {
+ val |= APIC_LVT_MASKED;
+ }
s->lvt[n] = val;
if (n == APIC_LVT_TIMER) {
apic_timer_update(s, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
--
2.37.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] hw/intc: Handle software disabling of APIC correctly
2022-07-12 14:18 [PATCH] hw/intc: Handle software disabling of APIC correctly Jay Khandkar
@ 2022-07-29 16:45 ` Jay Khandkar
2022-07-29 17:09 ` Peter Maydell
1 sibling, 0 replies; 6+ messages in thread
From: Jay Khandkar @ 2022-07-29 16:45 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, mst
[-- Attachment #1: Type: text/plain, Size: 1750 bytes --]
Ping?
On Tue, 12 Jul 2022, 19:49 Jay Khandkar, <jaykhandkar2002@gmail.com> wrote:
> When the local APIC is in a software disabled state, all local interrupt
> sources must be masked and all attempts to unmask them should be
> ignored. Currently, we don't do either. Fix this by handling it
> correctly in apic_mem_write().
>
> Signed-off-by: Jay Khandkar <jaykhandkar2002@gmail.com>
> ---
> hw/intc/apic.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> index 3df11c34d6..493c70af62 100644
> --- a/hw/intc/apic.c
> +++ b/hw/intc/apic.c
> @@ -792,9 +792,16 @@ static void apic_mem_write(void *opaque, hwaddr addr,
> uint64_t val,
> s->dest_mode = val >> 28;
> break;
> case 0x0f:
> - s->spurious_vec = val & 0x1ff;
> - apic_update_irq(s);
> - break;
> + {
> + s->spurious_vec = val & 0x1ff;
> + if (!(val & APIC_SPURIO_ENABLED)) {
> + for (int i = 0; i < APIC_LVT_NB; i++) {
> + s->lvt[i] |= APIC_LVT_MASKED;
> + }
> + }
> + apic_update_irq(s);
> + break;
> + }
> case 0x10 ... 0x17:
> case 0x18 ... 0x1f:
> case 0x20 ... 0x27:
> @@ -812,6 +819,9 @@ static void apic_mem_write(void *opaque, hwaddr addr,
> uint64_t val,
> case 0x32 ... 0x37:
> {
> int n = index - 0x32;
> + if (!(s->spurious_vec & APIC_SPURIO_ENABLED)) {
> + val |= APIC_LVT_MASKED;
> + }
> s->lvt[n] = val;
> if (n == APIC_LVT_TIMER) {
> apic_timer_update(s,
> qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> --
> 2.37.0
>
>
[-- Attachment #2: Type: text/html, Size: 2445 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] hw/intc: Handle software disabling of APIC correctly
2022-07-12 14:18 [PATCH] hw/intc: Handle software disabling of APIC correctly Jay Khandkar
2022-07-29 16:45 ` Jay Khandkar
@ 2022-07-29 17:09 ` Peter Maydell
2022-07-29 17:34 ` Jay Khandkar
1 sibling, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2022-07-29 17:09 UTC (permalink / raw)
To: Jay Khandkar; +Cc: qemu-devel, pbonzini, mst
On Tue, 12 Jul 2022 at 19:38, Jay Khandkar <jaykhandkar2002@gmail.com> wrote:
>
> When the local APIC is in a software disabled state, all local interrupt
> sources must be masked and all attempts to unmask them should be
> ignored. Currently, we don't do either. Fix this by handling it
> correctly in apic_mem_write().
>
> Signed-off-by: Jay Khandkar <jaykhandkar2002@gmail.com>
> ---
> hw/intc/apic.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> index 3df11c34d6..493c70af62 100644
> --- a/hw/intc/apic.c
> +++ b/hw/intc/apic.c
> @@ -792,9 +792,16 @@ static void apic_mem_write(void *opaque, hwaddr addr, uint64_t val,
> s->dest_mode = val >> 28;
> break;
> case 0x0f:
> - s->spurious_vec = val & 0x1ff;
> - apic_update_irq(s);
> - break;
> + {
> + s->spurious_vec = val & 0x1ff;
> + if (!(val & APIC_SPURIO_ENABLED)) {
> + for (int i = 0; i < APIC_LVT_NB; i++) {
> + s->lvt[i] |= APIC_LVT_MASKED;
> + }
> + }
> + apic_update_irq(s);
> + break;
> + }
What are the braces for here ? There's no local variable declaration...
thanks
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] hw/intc: Handle software disabling of APIC correctly
2022-07-29 17:09 ` Peter Maydell
@ 2022-07-29 17:34 ` Jay Khandkar
2022-08-17 10:50 ` Michael S. Tsirkin
0 siblings, 1 reply; 6+ messages in thread
From: Jay Khandkar @ 2022-07-29 17:34 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, pbonzini, mst
On Fri, Jul 29, 2022 at 06:09:01PM +0100, Peter Maydell wrote:
> On Tue, 12 Jul 2022 at 19:38, Jay Khandkar <jaykhandkar2002@gmail.com> wrote:
> >
> > When the local APIC is in a software disabled state, all local interrupt
> > sources must be masked and all attempts to unmask them should be
> > ignored. Currently, we don't do either. Fix this by handling it
> > correctly in apic_mem_write().
> >
> > Signed-off-by: Jay Khandkar <jaykhandkar2002@gmail.com>
> > ---
> > hw/intc/apic.c | 16 +++++++++++++---
> > 1 file changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> > index 3df11c34d6..493c70af62 100644
> > --- a/hw/intc/apic.c
> > +++ b/hw/intc/apic.c
> > @@ -792,9 +792,16 @@ static void apic_mem_write(void *opaque, hwaddr addr, uint64_t val,
> > s->dest_mode = val >> 28;
> > break;
> > case 0x0f:
> > - s->spurious_vec = val & 0x1ff;
> > - apic_update_irq(s);
> > - break;
> > + {
> > + s->spurious_vec = val & 0x1ff;
> > + if (!(val & APIC_SPURIO_ENABLED)) {
> > + for (int i = 0; i < APIC_LVT_NB; i++) {
> > + s->lvt[i] |= APIC_LVT_MASKED;
> > + }
> > + }
> > + apic_update_irq(s);
> > + break;
> > + }
>
> What are the braces for here ? There's no local variable declaration...
>
> thanks
> -- PMM
You are right, the braces are unnecessary for that part. I just put them in to
create a neat visually separate block. Can get rid of them.
Thanks,
Jay
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] hw/intc: Handle software disabling of APIC correctly
2022-07-29 17:34 ` Jay Khandkar
@ 2022-08-17 10:50 ` Michael S. Tsirkin
2022-09-07 17:59 ` Jay Khandkar
0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2022-08-17 10:50 UTC (permalink / raw)
To: Jay Khandkar; +Cc: Peter Maydell, qemu-devel, pbonzini
On Fri, Jul 29, 2022 at 11:04:47PM +0530, Jay Khandkar wrote:
> On Fri, Jul 29, 2022 at 06:09:01PM +0100, Peter Maydell wrote:
> > On Tue, 12 Jul 2022 at 19:38, Jay Khandkar <jaykhandkar2002@gmail.com> wrote:
> > >
> > > When the local APIC is in a software disabled state, all local interrupt
> > > sources must be masked and all attempts to unmask them should be
> > > ignored. Currently, we don't do either. Fix this by handling it
> > > correctly in apic_mem_write().
> > >
> > > Signed-off-by: Jay Khandkar <jaykhandkar2002@gmail.com>
> > > ---
> > > hw/intc/apic.c | 16 +++++++++++++---
> > > 1 file changed, 13 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> > > index 3df11c34d6..493c70af62 100644
> > > --- a/hw/intc/apic.c
> > > +++ b/hw/intc/apic.c
> > > @@ -792,9 +792,16 @@ static void apic_mem_write(void *opaque, hwaddr addr, uint64_t val,
> > > s->dest_mode = val >> 28;
> > > break;
> > > case 0x0f:
> > > - s->spurious_vec = val & 0x1ff;
> > > - apic_update_irq(s);
> > > - break;
> > > + {
> > > + s->spurious_vec = val & 0x1ff;
> > > + if (!(val & APIC_SPURIO_ENABLED)) {
> > > + for (int i = 0; i < APIC_LVT_NB; i++) {
> > > + s->lvt[i] |= APIC_LVT_MASKED;
> > > + }
> > > + }
> > > + apic_update_irq(s);
> > > + break;
> > > + }
> >
> > What are the braces for here ? There's no local variable declaration...
> >
> > thanks
> > -- PMM
> You are right, the braces are unnecessary for that part. I just put them in to
> create a neat visually separate block. Can get rid of them.
>
> Thanks,
> Jay
Did you intend to send v2 of this?
--
MST
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] hw/intc: Handle software disabling of APIC correctly
2022-08-17 10:50 ` Michael S. Tsirkin
@ 2022-09-07 17:59 ` Jay Khandkar
0 siblings, 0 replies; 6+ messages in thread
From: Jay Khandkar @ 2022-09-07 17:59 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Peter Maydell, qemu-devel, pbonzini
[-- Attachment #1: Type: text/plain, Size: 1971 bytes --]
Have sent in v2.
On Wed, Aug 17, 2022 at 4:20 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> On Fri, Jul 29, 2022 at 11:04:47PM +0530, Jay Khandkar wrote:
> > On Fri, Jul 29, 2022 at 06:09:01PM +0100, Peter Maydell wrote:
> > > On Tue, 12 Jul 2022 at 19:38, Jay Khandkar <jaykhandkar2002@gmail.com>
> wrote:
> > > >
> > > > When the local APIC is in a software disabled state, all local
> interrupt
> > > > sources must be masked and all attempts to unmask them should be
> > > > ignored. Currently, we don't do either. Fix this by handling it
> > > > correctly in apic_mem_write().
> > > >
> > > > Signed-off-by: Jay Khandkar <jaykhandkar2002@gmail.com>
> > > > ---
> > > > hw/intc/apic.c | 16 +++++++++++++---
> > > > 1 file changed, 13 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> > > > index 3df11c34d6..493c70af62 100644
> > > > --- a/hw/intc/apic.c
> > > > +++ b/hw/intc/apic.c
> > > > @@ -792,9 +792,16 @@ static void apic_mem_write(void *opaque, hwaddr
> addr, uint64_t val,
> > > > s->dest_mode = val >> 28;
> > > > break;
> > > > case 0x0f:
> > > > - s->spurious_vec = val & 0x1ff;
> > > > - apic_update_irq(s);
> > > > - break;
> > > > + {
> > > > + s->spurious_vec = val & 0x1ff;
> > > > + if (!(val & APIC_SPURIO_ENABLED)) {
> > > > + for (int i = 0; i < APIC_LVT_NB; i++) {
> > > > + s->lvt[i] |= APIC_LVT_MASKED;
> > > > + }
> > > > + }
> > > > + apic_update_irq(s);
> > > > + break;
> > > > + }
> > >
> > > What are the braces for here ? There's no local variable declaration...
> > >
> > > thanks
> > > -- PMM
> > You are right, the braces are unnecessary for that part. I just put them
> in to
> > create a neat visually separate block. Can get rid of them.
> >
> > Thanks,
> > Jay
>
> Did you intend to send v2 of this?
>
> --
> MST
>
>
[-- Attachment #2: Type: text/html, Size: 3012 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-07 18:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-12 14:18 [PATCH] hw/intc: Handle software disabling of APIC correctly Jay Khandkar
2022-07-29 16:45 ` Jay Khandkar
2022-07-29 17:09 ` Peter Maydell
2022-07-29 17:34 ` Jay Khandkar
2022-08-17 10:50 ` Michael S. Tsirkin
2022-09-07 17:59 ` Jay Khandkar
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.