Re: [Xen-devel] [PATCH v2 4/9] xen: arm: correctly handle vtimer traps from userspace

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 19/02/15 12:10, Ian Campbell wrote:
> On Tue, 2015-02-10 at 14:41 +0800, Julien Grall wrote:
>> Hi Ian,
>>
>> On 10/02/2015 12:45, Ian Campbell wrote:
>>> Previously 32-bit userspace on 32-bit kernel and 64-bit userspace on 64-bit
>>> kernel could access these registers irrespective of whether the kernel had
>>> configured them to be allowed to. To fix this:
>>>
>>>   - Userspace access to CNTP_CTL_EL0 and CNTP_TVAL_EL0 should be gated on
>>>     CNTKCTL_EL1.EL0PTEN.
>>
>> Should not we take care of CNTP_CVAL_EL0 too? It seems that we don't 
>> even trap it for now...
> 
> We should, I'll prepend such a patch to the series, since it should be
> backported.
> 
> As it happens Linux doesn't allow user access to this register (due to
> the settings in CNTKCTL), so it traps to EL1 in h/w and we never see it.
> But if an OS were to expose the phys timer to userspace for some reason
> then we would trap and inject undef, which is a bit mean of us.

Good, thanks!

>>
>> [..]
>>
>>> @@ -2062,8 +2053,7 @@ asmlinkage void do_trap_hypervisor(struct 
>>> cpu_user_regs *regs)
>>>           do_cp15_32(regs, hsr);
>>>           break;
>>>       case HSR_EC_CP15_64:
>>> -        if ( !is_32bit_domain(current->domain) )
>>> -            goto bad_trap;
>>> +        BUG_ON(!psr_mode_is_32bit(regs->cpsr));
>>
>> You should mention the change from if ( .... ) goto bad_trap to BUG_ON( 
>> ... ) in the commit message.
> 
> OK.
> 
>> Although, I think the debug message in bad_trap is useful to keep. It 
>> may be handy to have the HSR and the guest stack trace printed if Xen 
>> hit the condition.
> 
> Doesn't BUG_ON include all that? It should really.

Not really BUG_ON will jump into the exception mode and therefore print
the HSR of the exception (breakpoint for ARM64 and undef for ARM32).

Also, BUG_ON doesn't print the stack trace of the guest but only the
hypervisor one. The latter is not really useful in the current case.

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel