Re: [PATCH v2] ignore writes to GICD_ICACTIVER ... GICD_ICACTIVERN

[Julien Grall <julien.grall@citrix.com>]

Hi Stefano,

On 11/12/15 14:36, Stefano Stabellini wrote:
> On Fri, 4 Dec 2015, Julien Grall wrote:
>> On 04/12/2015 12:36, 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>
>>>
>>> ---
>>>
>>> Changes in v2:
>>> - rebase on staging
>>>
>>> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
>>> index 2c73133..3079901 100644
>>> --- a/xen/arch/arm/vgic-v2.c
>>> +++ b/xen/arch/arm/vgic-v2.c
>>> @@ -494,11 +494,10 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v,
>>> mmio_info_t *info,
>>>           return 0;
>>>
>>>       case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
>>> -        if ( dabt.size != DABT_WORD ) goto bad_width;
>>> -        printk(XENLOG_G_ERR
>>> +        gdprintk(XENLOG_DEBUG,
>>
>> Why did you  move to gdprintk? The vCPU is already printed using %pv in the
>> string.
> 
> Because this is a log message which is directly caused by guest
> behaviour, so it makes more sense to be to be a gdprintk

gdprintk will print the vCPU and so does %pv. What's the point to print
it two times?

Furthermore, compare to before gdprintk will be dropped on non-debug
build. It looks like to me this message would still be useful in
non-debug build. You also move from G_ERR to DEBUG, although it's still
an error because we don't emulate properly the register.

Finally, we decided to use printk rather than g*printk because it prints
unnecessary information such as filename and line (see [1])

So please stay consistent with the rest of the emulation.

Regards,

[1] c96222cc6dbb285a4de8f25e3b8e284e212ef964 "xen/arm: vgic-v3: message
in the emulation code should be rate-limited"

-- 
Julien Grall