Re: [PATCH] vm_event: Implement ARM SMC events

From: Corneliu ZUZU <czuzu@bitdefender.com>
To: Julien Grall <julien.grall@arm.com>,
	Tamas K Lengyel <tamas.k.lengyel@gmail.com>
Cc: Wei Liu <wei.liu2@citrix.com>, Keir Fraser <keir@xen.org>,
	Razvan Cojocaru <rcojocaru@bitdefender.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	Jan Beulich <jbeulich@suse.com>,
	Xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH] vm_event: Implement ARM SMC events
Date: Tue, 12 Apr 2016 20:05:07 +0300	[thread overview]
Message-ID: <570D2AC3.2040801@bitdefender.com> (raw)
In-Reply-To: <570D2135.1040204@arm.com>

On 4/12/2016 7:24 PM, Julien Grall wrote:
> Hello,
>
> On 12/04/2016 16:01, Tamas K Lengyel wrote:
>>
>> On Apr 12, 2016 01:51, "Corneliu ZUZU" <czuzu@bitdefender.com
>> <mailto:czuzu@bitdefender.com>> wrote:
>>  >
>>  > On 4/11/2016 10:47 PM, Tamas K Lengyel wrote:
>>  >>
>>  >> From: Tamas K Lengyel <tklengyel@sec.in.tum.de
>> <mailto:tklengyel@sec.in.tum.de>>
>>  >>
>>  >> The ARM SMC instructions are already configured to trap to Xen by
>> default. In
>>  >> this patch we allow a user-space process in a privileged domain to
>> receive
>>  >> notification of when such event happens through the vm_event 
>> subsystem.
>>  >>
>>  >> This patch will likely needs to be broken up into several smaller
>> patches.
>>  >> Right now what this patch adds (and could be broken into smaller 
>> patches
>>  >> accordingly):
>>  >>      - Implement monitor_op domctl handler for SOFTWARE_BREAKPOINTs
>> on ARM
>>  >>      - Implement vm_event register fill/set routines for ARM. This
>> required
>>  >>          removing the function from common as the function 
>> prototype now
>>  >>          differs on the two archs.
>>  >>      - Sending notification as SOFTWARE_BREAKPOINT vm_event from the
>> SMC trap
>>  >>          handlers.
>>  >>      - Extend the xen-access test tool to receive SMC notification
>> and step
>>  >>          the PC manually in the reply.
>>  >>
>>  >> I'm sending it as an RFC to gather feedback on what has been
>> overlooked in this
>>  >> revision. This patch has been tested on a Cubietruck board and works
>> fine,
>>  >> but would probably not work on 64-bit boards.
>>  >
>>  >
>>  > Hi Tamas,
>>  >
>>  > If I may, I'm still unable to work at the moment, being ill, but I'm
>> checking the xen-devel lists from time to time.
>>  > Your patch caught my attention, reminding me of the conversation we
>> had some time ago on this matter.
>>  > The only real reason I don't see SMC (secure-monitor-call) as being
>> an ideal candidate for this is that, according to the ARM manuals, SMC
>> should directly cause undefined exception if executed from user-mode
>> (EL0), instead of a hypervisor trap - isn't that the case on the machine
>> you tested this on or is this really only for the EL1 of domains?
>
> This paragraph is part of Corneliu's answer but it gives the 
> impression you wrote it. Can you configure your e-mail client to quote 
> properly?
>
>>
>> That's correct, it can only be issued by the kernel. So as long as you
>> want to monitor the kernel it can be used just fine. I can also envision
>> trampoline-like traps (syscalls injected into EL0 to trigger SMC) but
>> that's beyond the scope I intend this for now.

Then indeed SMC is the -easiest- way to go, @ least until user-mode 
breakpoints are implemented.

>>
>>  >
>>  > Also:
>>  > - SMC, by definition, is a call to the secure side, it doesn't relate
>> to debugging directly (it's a syscall to the 'secure' side). There is a
>> viable INT3 equivalent on ARM, that being the BKPT/BRK instruction,
>> using that instead would require a bit more effort (but would,
>> conceptually, be more correct) and might be less performant, I suppose
>> that's why you didn't go for that?
>
> BKPT/BRK could be used by the guest for debugging. You would need to 
> differentiate a breakpoint inserted by Xen or by a debugger in the guest.

Isn't that also the case for X86's INT3? I guess differentiation is done 
based on the bkpt address/privilege level? On ARM that could also 
(partially) be done by looking @ the immediate value of the BKPT/BRK 
instruction. Another thing I realized might be troublesome with NOT 
using BKPT/BRK would be that on ARMv7 BKPT is always unconditional, even 
in IT blocks. IDK if that applies to SMC, but if it doesn't you'd have 
to check for that as well to make sure the breakpoint is actually executed.

>
>>
>> I would have to double check but AFAIK those instructions can't be
>> configured to trap to the hypervisor directly. So while SMC was not
>> intended to be a breakpoint, conceptually it's the closest thing we have
>> an on ARM to the INT3 instruction when configured to trap to the VMM.
>

Please see AArch32 HDCR.TDE and AArch64 MDCR_EL2.TDE bits. Since 
activating this bit would imply additional (in this context -unwanted-) 
traps, the performance hit of having this bit set might be significant.

> Whilst any access to SMC currently results to inject an undefined 
> exception, it may not be the case in the future. There have been 
> discussion to allow guest issuing SMC call (see [1]).
>
> I think the safest instruction would be HVC #imm. Xen is only using a 
> small number of immediate. You could allocate a specific value for 
> software debugging.
>

IMHO that would also be better conceptually, although it would still 
suffer from the limitation of not being available from user-space (and 
potentially from the above IT block issue).

>>
>>  > - SMC can be disabled by the secure side (over which Xen doesn't have
>> control) - not really a problem on though, since the hypervisor trap
>> happens before that check
>>  > But these 2 are conceptual problems, they don't impede usage of SMC
>> as you intend in practice.
>>
>> Sure, the TrustZone is more privileged then the hypervisor so you need
>> to take that into account as well when you consider your threat model.
>> If the TZ is malicious though IMHO there isn't much you can do on the
>> hypervisor side anyway. So in the usecase I have for this I control the
>> TZ as well.
>
> Regards,
>
> [1] 
> http://lists.xen.org/archives/html/xen-devel/2015-07/txtwZfvJnXlYG.txt
>

Regards,
Corneliu.

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