Re: SEV guest debugging support for Qemu

[Ashish Kalra <ashish.kalra@amd.com>]

Hello Dave,

Thanks for your response, please see my replies inline :

On Thu, Sep 24, 2020 at 02:53:42PM +0100, Dr. David Alan Gilbert wrote:
> * Ashish Kalra (ashish.kalra@amd.com) wrote:
> > Hello Alan, Paolo,
> > 
> > I am following up on Brijesh’s patches for SEV guest debugging support for Qemu using gdb and/or qemu monitor.
> > I believe that last time, Qemu SEV debug patches were not applied and have attached the link to the email thread and Paolo’s feedback below for reference [1].
> > I wanted to re-start a discussion on the same here with the Qemu community and seek the feedback on the approaches which we are considering :
> > Looking at Qemu code, I see the following interface is defined, for virtual memory access for debug : cpu_memory_rw_debug(). 
> > Both gdbstub (target_memory_rw_debug() ) and QMP/HMP (monitor/misc.c : memory_dump() ) use this standard and well-defined interface to access guest memory for debugging purposes. 
> > 
> > This internally invokes the address_space_rw() accessor functions which we had  "fixed" internally (as part of the earlier patch) to invoke memory region specific debug ops. 
> > In our earlier approach we were adding debug ops/callbacks to memory regions and as per comments on our earlier patches, Paolo was not happy with this debug API for
> > MemoryRegions and hence the SEV support for Qemu was merged without the debug support.
> > 
> > Now, we want to reuse this cpu_memory_rw_debug() interface or alternatively introduce a new generic debug interface/object in the Qemu. This 
> > debug interface should be controlled through the global machine policy.
> 
> Let me leave the question of how the memory_rw_debug interface should
> work to Paolo.
> 
> > For e.g., 
> > # $QEMU -machine -debug=<a debug object>
> > or
> > # $QEMU -machine -debug=sev-guest-debug
> > 
> > The QMP and GDB access will be updated to use the generic debug  interface. The generic debug interface or the cpu_memory_rw_debug() interace will introduce hooks to call a 
> > vendor specific debug object to delegate accessing the data. The vendor specific debug object may do a further checks before and after accessing the memory.
> 
> I'm not sure that needs a commandline switch for it; since you can
> already get it from the guest policy in the sev object and I can't think
> of any other cases that would need something similar.

Yes, i agree with that, so i am now considering abstracting this vendor
specific debug interface via CPUClass object instead of doing it via
MemoryRegions. 

> > Now, looking specifically at cpu_memory_rw_debug() interface, this interface is invoked for all guest memory accesses for debugging purposes and it also does 
> > guest VA to GPA translation via cpu_get_phys_page_attrs_debug(), so we can again add a vendor specific callback here to do guest VA to GPA translations specific
> > to SEV as SEV guest debugging will also require accessing guest page table entries and decrypting them via the SEV DBG_DECRYPT APIs and additionally clearing
> > the C-bit on page table entries (PxEs) before using them further for page table walks.
> > 
> > There is still an issue with the generic cpu_memory_rw_debug() interface, though it is used for all guest memory accesses for debugging and we can also handle
> > guest page table walks via it (as mentioned above), there are still other gdb/monitor commands such as tlb_info_xx() and mem_info_xx() which also do guest page
> > table walks, but they don’t go through any generic guest memory access/debug interface, so these commands will need to be handled additionally for SEV.
> 
> If some of those should be using the debug interface and aren't then
> please fix them anyway.
> 
> > The vendor specific debug object (added as a hook to generic debug object or the generic cpu_memory_rw_debug() interface) will do further checks before and after accessing the memory.
> > 
> > e.g., in the case of SEV,
> > 
> > 1. Check the guest policy, if guest policy does not allow debug then return an error.
> > 
> > 2. If its an MMIO region then access the data.
> > 
> > 3. If its RAM region then call the PSP commands to decrypt the data.
> > 
> > 4. If caller asked to read the PTE entry then probably clear the C-bits after reading the PTE entry.
> 
> Does that work if the guest is currently running?
> 

I assume you are asking that is this done when guest is being debugged,
the above steps are only done when the guest is paused and being debugged.

Thanks,
Ashish

> 
> > 5. many more checks
> > 
> > Looking fwd. to your feedback/comments on the above approach or other any other suggestions.
> > 
> > Thanks,
> > Ashish
> > 
> > [1] -> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fnext.patchew.org%2FQEMU%2F20180308124901.83533-1-brijesh.singh%40amd.com%2F20180308124901.83533-29-brijesh.singh%40amd.com%2F&amp;data=02%7C01%7Cashish.kalra%40amd.com%7Cd21e40d3527d4dba609c08d86091490e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637365524404435805&amp;sdata=P%2F6DqPQmUObJipkbbeXcrUdCqulePiqxSU6OB8xUEWo%3D&amp;reserved=0
> > 
> -- 
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>