Re: [PATCH] arm/vgic-v3: Fix write_ignore_64's check in __vgic_v3_rdistr_rd_mmio_write()
On 20.05.25 18:02, Julien Grall wrote:
> Hi Oleksandr,
Hello Julien
>
> On 20/05/2025 14:47, Oleksandr Tyshchenko wrote:
>> An attempt to write access the register (i.e. GICR_PROPBASER,
>> GICR_PENDBASER)
>> which should be ignored (i.e. no virtual ITS present) causes the data
>> about
>
> I assume, this is a guest data abort, rather than Xen crash?
yes
>
>> due to incorrect check at the write_ignore_64 label. The check should be
>> inverted.
>
> OOI, why would a guest try to write to GICR_PROPBASER if the ITS is not
> present? Was it a bug in the OS?
no, it was just me experimenting with redistributor registers.
>
>>
>> Fixes: c4d6bbdc12e5 ("xen/arm: vgic-v3: Support 32-bit access for
>> 64-bit registers")
>> Signed-off-by: Oleksandr Tyshchenko
>
> With the commit message clarified and Andrew's comments addressed:
>
> Acked-by: Julien Grall
thanks
>
> Cheers,
>
Re: [PATCH] arm/vgic-v3: Fix write_ignore_64's check in __vgic_v3_rdistr_rd_mmio_write()
On 20.05.25 17:24, Andrew Cooper wrote:
Hello Andrew
> On 20/05/2025 2:47 pm, Oleksandr Tyshchenko wrote:
>> An attempt to write access the register (i.e. GICR_PROPBASER, GICR_PENDBASER)
>> which should be ignored (i.e. no virtual ITS present) causes the data about
>
> Do you mean "data abort" here?
yes
If not, I can't parse the sentence.
>
>> due to incorrect check at the write_ignore_64 label. The check should be
>> inverted.
>>
>> Fixes: c4d6bbdc12e5 ("xen/arm: vgic-v3: Support 32-bit access for 64-bit
>> registers")
>> Signed-off-by: Oleksandr Tyshchenko
>> ---
>> xen/arch/arm/vgic-v3.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
>> index 2eaa48fadb..b366b046a2 100644
>> --- a/xen/arch/arm/vgic-v3.c
>> +++ b/xen/arch/arm/vgic-v3.c
>> @@ -649,7 +649,7 @@ bad_width:
>> return 0;
>>
>> write_ignore_64:
>> -if ( vgic_reg64_check_access(dabt) ) goto bad_width;
>> +if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
>
> As you're modifying anyway, the goto should be on the next line.
ok, will move
>
> ~Andrew
Re: [PATCH] arm/vgic-v3: Fix write_ignore_64's check in __vgic_v3_rdistr_rd_mmio_write()
Hi Oleksandr,
On 20/05/2025 14:47, Oleksandr Tyshchenko wrote:
An attempt to write access the register (i.e. GICR_PROPBASER, GICR_PENDBASER)
which should be ignored (i.e. no virtual ITS present) causes the data about
I assume, this is a guest data abort, rather than Xen crash?
due to incorrect check at the write_ignore_64 label.
The check should be
inverted.
OOI, why would a guest try to write to GICR_PROPBASER if the ITS is not
present? Was it a bug in the OS?
Fixes: c4d6bbdc12e5 ("xen/arm: vgic-v3: Support 32-bit access for 64-bit
registers")
Signed-off-by: Oleksandr Tyshchenko
With the commit message clarified and Andrew's comments addressed:
Acked-by: Julien Grall
Cheers,
--
Julien Grall
Re: [PATCH] arm/vgic-v3: Fix write_ignore_64's check in __vgic_v3_rdistr_rd_mmio_write()
On 20/05/2025 2:47 pm, Oleksandr Tyshchenko wrote:
> An attempt to write access the register (i.e. GICR_PROPBASER, GICR_PENDBASER)
> which should be ignored (i.e. no virtual ITS present) causes the data about
Do you mean "data abort" here? If not, I can't parse the sentence.
> due to incorrect check at the write_ignore_64 label. The check should be
> inverted.
>
> Fixes: c4d6bbdc12e5 ("xen/arm: vgic-v3: Support 32-bit access for 64-bit
> registers")
> Signed-off-by: Oleksandr Tyshchenko
> ---
> xen/arch/arm/vgic-v3.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index 2eaa48fadb..b366b046a2 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -649,7 +649,7 @@ bad_width:
> return 0;
>
> write_ignore_64:
> -if ( vgic_reg64_check_access(dabt) ) goto bad_width;
> +if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
As you're modifying anyway, the goto should be on the next line.
~Andrew
