Re: SEV guest debugging support for Qemu

[Brijesh Singh]

On 9/24/20 2:06 PM, Ashish Kalra wrote:
> 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.


I don't why we need to pause the guest. Ideally we should be able to
connect to Qemu monitor and run the "x" command to dump memory. IIRC, if
paging is enabled then monitor will walk the guest page table to reach
to gpa. Something like this in the Qemu monitor console should work:

x /10i $eip


> 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
>>