* [PATCH] ignore writes to GICD_ICACTIVER ... GICD_ICACTIVERN
@ 2015-12-03 11:02 Stefano Stabellini
2015-12-03 11:25 ` Julien Grall
2015-12-03 11:41 ` Jan Beulich
0 siblings, 2 replies; 6+ messages in thread
From: Stefano Stabellini @ 2015-12-03 11:02 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, Ian Campbell, Stefano Stabellini
Injecting a fault to the guest just because it is writing to one of the
GICD_ICACTIVER registers, which are part of the GICv2 and GICv3 specs,
is harsh. Additionally it causes recent linux kernels to fail to boot on
Xen.
Ignore writes to GICD_ICACTIVER ... GICD_ICACTIVERN instead, to solve
the boot issue and for backportability. However implementing the
registers properly might a better long term solution.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index b5a8f29..bf9f239 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -315,11 +315,10 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
return 0;
case GICD_ICACTIVER ... GICD_ICACTIVERN:
- if ( dabt.size != DABT_WORD ) goto bad_width;
- printk(XENLOG_G_ERR
+ gdprintk(XENLOG_DEBUG,
"%pv: vGICD: unhandled word write %#"PRIregister" to ICACTIVER%d\n",
v, *r, gicd_reg - GICD_ICACTIVER);
- return 0;
+ goto write_ignore_32;
case GICD_ITARGETSR ... GICD_ITARGETSR + 7:
/* SGI/PPI target is read only */
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 45a46c3..e115027 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -412,11 +412,10 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
return 0;
case GICD_ICACTIVER ... GICD_ICACTIVERN:
- if ( dabt.size != DABT_WORD ) goto bad_width;
- printk(XENLOG_G_ERR
+ gdprintk(XENLOG_DEBUG,
"%pv: %s: unhandled word write %#"PRIregister" to ICACTIVER%d\n",
v, name, *r, reg - GICD_ICACTIVER);
- return 0;
+ goto write_ignore_32;
case GICD_IPRIORITYR ... GICD_IPRIORITYRN:
if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] ignore writes to GICD_ICACTIVER ... GICD_ICACTIVERN
2015-12-03 11:02 [PATCH] ignore writes to GICD_ICACTIVER ... GICD_ICACTIVERN Stefano Stabellini
@ 2015-12-03 11:25 ` Julien Grall
2015-12-03 11:41 ` Jan Beulich
1 sibling, 0 replies; 6+ messages in thread
From: Julien Grall @ 2015-12-03 11:25 UTC (permalink / raw)
To: Stefano Stabellini, xen-devel; +Cc: Ian Campbell
Hi Stefano,
On 03/12/15 11:02, Stefano Stabellini wrote:
> Injecting a fault to the guest just because it is writing to one of the
> GICD_ICACTIVER registers, which are part of the GICv2 and GICv3 specs,
> is harsh. Additionally it causes recent linux kernels to fail to boot on
> Xen.
>
> Ignore writes to GICD_ICACTIVER ... GICD_ICACTIVERN instead, to solve
> the boot issue and for backportability. However implementing the
> registers properly might a better long term solution.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Please rebase this patch on top of staging.
Regards,
>
> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
> index b5a8f29..bf9f239 100644
> --- a/xen/arch/arm/vgic-v2.c
> +++ b/xen/arch/arm/vgic-v2.c
> @@ -315,11 +315,10 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
> return 0;
>
> case GICD_ICACTIVER ... GICD_ICACTIVERN:
> - if ( dabt.size != DABT_WORD ) goto bad_width;
> - printk(XENLOG_G_ERR
> + gdprintk(XENLOG_DEBUG,
> "%pv: vGICD: unhandled word write %#"PRIregister" to ICACTIVER%d\n",
> v, *r, gicd_reg - GICD_ICACTIVER);
> - return 0;
> + goto write_ignore_32;
>
> case GICD_ITARGETSR ... GICD_ITARGETSR + 7:
> /* SGI/PPI target is read only */
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index 45a46c3..e115027 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -412,11 +412,10 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
> return 0;
>
> case GICD_ICACTIVER ... GICD_ICACTIVERN:
> - if ( dabt.size != DABT_WORD ) goto bad_width;
> - printk(XENLOG_G_ERR
> + gdprintk(XENLOG_DEBUG,
> "%pv: %s: unhandled word write %#"PRIregister" to ICACTIVER%d\n",
> v, name, *r, reg - GICD_ICACTIVER);
> - return 0;
> + goto write_ignore_32;
>
> case GICD_IPRIORITYR ... GICD_IPRIORITYRN:
> if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
--
Julien Grall
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ignore writes to GICD_ICACTIVER ... GICD_ICACTIVERN
2015-12-03 11:02 [PATCH] ignore writes to GICD_ICACTIVER ... GICD_ICACTIVERN Stefano Stabellini
2015-12-03 11:25 ` Julien Grall
@ 2015-12-03 11:41 ` Jan Beulich
2015-12-03 11:41 ` Julien Grall
1 sibling, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2015-12-03 11:41 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: Julien Grall, xen-devel, Ian Campbell
>>> On 03.12.15 at 12:02, <stefano.stabellini@eu.citrix.com> wrote:
> --- a/xen/arch/arm/vgic-v2.c
> +++ b/xen/arch/arm/vgic-v2.c
> @@ -315,11 +315,10 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
> return 0;
>
> case GICD_ICACTIVER ... GICD_ICACTIVERN:
> - if ( dabt.size != DABT_WORD ) goto bad_width;
> - printk(XENLOG_G_ERR
> + gdprintk(XENLOG_DEBUG,
> "%pv: vGICD: unhandled word write %#"PRIregister" to ICACTIVER%d\n",
> v, *r, gicd_reg - GICD_ICACTIVER);
Do you really mean to print two domain/vcpu ID pairs here, the more
that I suppose v == current?
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ignore writes to GICD_ICACTIVER ... GICD_ICACTIVERN
2015-12-03 11:41 ` Jan Beulich
@ 2015-12-03 11:41 ` Julien Grall
2015-12-03 12:47 ` Jan Beulich
0 siblings, 1 reply; 6+ messages in thread
From: Julien Grall @ 2015-12-03 11:41 UTC (permalink / raw)
To: Jan Beulich, Stefano Stabellini; +Cc: xen-devel, Ian Campbell
On 03/12/15 11:41, Jan Beulich wrote:
>>>> On 03.12.15 at 12:02, <stefano.stabellini@eu.citrix.com> wrote:
>> --- a/xen/arch/arm/vgic-v2.c
>> +++ b/xen/arch/arm/vgic-v2.c
>> @@ -315,11 +315,10 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
>> return 0;
>>
>> case GICD_ICACTIVER ... GICD_ICACTIVERN:
>> - if ( dabt.size != DABT_WORD ) goto bad_width;
>> - printk(XENLOG_G_ERR
>> + gdprintk(XENLOG_DEBUG,
>> "%pv: vGICD: unhandled word write %#"PRIregister" to ICACTIVER%d\n",
>> v, *r, gicd_reg - GICD_ICACTIVER);
>
> Do you really mean to print two domain/vcpu ID pairs here, the more
> that I suppose v == current?
v is not necessarily the current vCPU.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ignore writes to GICD_ICACTIVER ... GICD_ICACTIVERN
2015-12-03 11:41 ` Julien Grall
@ 2015-12-03 12:47 ` Jan Beulich
2015-12-03 12:52 ` Julien Grall
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2015-12-03 12:47 UTC (permalink / raw)
To: Julien Grall, Stefano Stabellini; +Cc: xen-devel, Ian Campbell
>>> On 03.12.15 at 12:41, <julien.grall@citrix.com> wrote:
> On 03/12/15 11:41, Jan Beulich wrote:
>>>>> On 03.12.15 at 12:02, <stefano.stabellini@eu.citrix.com> wrote:
>>> --- a/xen/arch/arm/vgic-v2.c
>>> +++ b/xen/arch/arm/vgic-v2.c
>>> @@ -315,11 +315,10 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v,
> mmio_info_t *info)
>>> return 0;
>>>
>>> case GICD_ICACTIVER ... GICD_ICACTIVERN:
>>> - if ( dabt.size != DABT_WORD ) goto bad_width;
>>> - printk(XENLOG_G_ERR
>>> + gdprintk(XENLOG_DEBUG,
>>> "%pv: vGICD: unhandled word write %#"PRIregister" to
> ICACTIVER%d\n",
>>> v, *r, gicd_reg - GICD_ICACTIVER);
>>
>> Do you really mean to print two domain/vcpu ID pairs here, the more
>> that I suppose v == current?
>
> v is not necessarily the current vCPU.
Okay, that's different then from how MMIO intercepts work on x86.
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ignore writes to GICD_ICACTIVER ... GICD_ICACTIVERN
2015-12-03 12:47 ` Jan Beulich
@ 2015-12-03 12:52 ` Julien Grall
0 siblings, 0 replies; 6+ messages in thread
From: Julien Grall @ 2015-12-03 12:52 UTC (permalink / raw)
To: Jan Beulich, Stefano Stabellini; +Cc: xen-devel, Ian Campbell
On 03/12/15 12:47, Jan Beulich wrote:
>>>> On 03.12.15 at 12:41, <julien.grall@citrix.com> wrote:
>> On 03/12/15 11:41, Jan Beulich wrote:
>>>>>> On 03.12.15 at 12:02, <stefano.stabellini@eu.citrix.com> wrote:
>>>> --- a/xen/arch/arm/vgic-v2.c
>>>> +++ b/xen/arch/arm/vgic-v2.c
>>>> @@ -315,11 +315,10 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v,
>> mmio_info_t *info)
>>>> return 0;
>>>>
>>>> case GICD_ICACTIVER ... GICD_ICACTIVERN:
>>>> - if ( dabt.size != DABT_WORD ) goto bad_width;
>>>> - printk(XENLOG_G_ERR
>>>> + gdprintk(XENLOG_DEBUG,
>>>> "%pv: vGICD: unhandled word write %#"PRIregister" to
>> ICACTIVER%d\n",
>>>> v, *r, gicd_reg - GICD_ICACTIVER);
>>>
>>> Do you really mean to print two domain/vcpu ID pairs here, the more
>>> that I suppose v == current?
>>
>> v is not necessarily the current vCPU.
>
> Okay, that's different then from how MMIO intercepts work on x86.
It really depends of the emulation. This code emulates the state of the
interrupt controller for a given vCPU.
For GICv2, v will always be the current vCPU because the region is
either common or banked.
However for GICv3, each vCPU has its own region (i.e the re-distributor)
and can be accessible by all the other vCPUs.
So the code will look up the vCPU associated to this region and will use
it in the retrieve the correct state (see vgic_v3_rdistr_mmio_{read,write}).
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-12-03 12:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-03 11:02 [PATCH] ignore writes to GICD_ICACTIVER ... GICD_ICACTIVERN Stefano Stabellini
2015-12-03 11:25 ` Julien Grall
2015-12-03 11:41 ` Jan Beulich
2015-12-03 11:41 ` Julien Grall
2015-12-03 12:47 ` Jan Beulich
2015-12-03 12:52 ` Julien Grall
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.