* Ashish Kalra (ashish.ka...@amd.com) wrote: > On Thu, Sep 24, 2020 at 02:37:33PM -0500, Brijesh Singh wrote: > > > > 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.ka...@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 > > > > > > Yes that works, what i basically meant that monitor will invoke a set of > debugging > interfaces to get gpa and then dump guest memory even while guest is > running.
OK, I was worried about the bit where you said 'clear the C-bits' - as long as that's just clearing it in the copy you've taken rather than the in memory version the guest is using then that's OK. Dave > 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&data=02%7C01%7Cashish.kalra%40amd.com%7Cd21e40d3527d4dba609c08d86091490e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637365524404435805&sdata=P%2F6DqPQmUObJipkbbeXcrUdCqulePiqxSU6OB8xUEWo%3D&reserved=0 > > >>> > > >> -- > > >> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK > > >> > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
