SEV guest debugging support for Qemu

SEV guest debugging support for Qemu

[Kalra, Ashish]

[-- Attachment #1: Type: text/plain, Size: 3775 bytes --]

[AMD Public Use]

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.



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.



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.



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.



5. many more checks



Looking fwd. to your feedback/comments on the above approach or other any other suggestions.



Thanks,

Ashish

[1] -> http://next.patchew.org/QEMU/20180308124901.83533-1-brijesh.singh@amd.com/20180308124901.83533-29-brijesh.singh@amd.com/

[-- Attachment #2: Type: text/html, Size: 10761 bytes --]

Re: SEV guest debugging support for Qemu

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1946 bytes --]

Il lun 28 set 2020, 15:26 Ashish Kalra <ashish.kalra@amd.com> ha scritto:

> Hello Paolo,
>
> On Sat, Sep 26, 2020 at 02:02:20AM +0200, Paolo Bonzini wrote:
> > On 26/09/20 01:48, Ashish Kalra wrote:
> > > Thanks for your input, i have one additional query with reference to
> this support :
> > >
> > > For all explicitly unecrypted guest memory regions such as S/W IOTLB
> bounce buffers,
> > > dma_decrypted() allocated regions and for guest regions marked as
> "__bss_decrypted",
> > > we need to ensure that DBG_DECRYPT API calls are bypassed for such
> > > regions and those regions are dumped as un-encrypted.
> >
> > Yes those would be a bit different as they would be physical memory
> > accesses.  Those currently go through address_space_read in memory_dump
> > (monitor/misc.c), and would have to use the MemoryDebugOps instead.
> > That is the place to hook into in order to read the KVM page encryption
> > bitmap (which is not per-CPU, so another MemoryDebugOps entry
> > get_phys_addr_attrs?); the MemTxAttrs can then be passed to the read
> > function in the MemoryDebugOps.
> >
>
> Actually, currently we do this in sev_dbg_crypt() in KVM itself by
> checking the page encryption bitmap and if it is an un-encrypted guest
> memory region then returning the un-encrypted buffer read from user
> back to it as it is.
>

Fair enough. :-)

Paolo


> > > This guest memory regions encryption status is found using KVM's page
> encryption bitmap
> > > support which is part of the page encryption bitmap hypercall
> interface of the
> > > KVM/QEMU SEV live migration patches.
> > >
> > > As this additional debug support is dependent on the KVM's page
> encryption bitmap
> > > support, are there any updates on KVM SEV live migration patches ?
> >
> > Sorry about that, I've been busy with QEMU.  I'll review them as soon as
> > possible.H
>
> Looking forward to your updates and reviews regarding the same.
>
> Thanks,
> Ashish
>
>

[-- Attachment #2: Type: text/html, Size: 2767 bytes --]

Re: SEV guest debugging support for Qemu

[Ashish Kalra]

Hello Paolo,

On Sat, Sep 26, 2020 at 02:02:20AM +0200, Paolo Bonzini wrote:
> On 26/09/20 01:48, Ashish Kalra wrote:
> > Thanks for your input, i have one additional query with reference to this support :
> > 
> > For all explicitly unecrypted guest memory regions such as S/W IOTLB bounce buffers,
> > dma_decrypted() allocated regions and for guest regions marked as "__bss_decrypted",
> > we need to ensure that DBG_DECRYPT API calls are bypassed for such
> > regions and those regions are dumped as un-encrypted.
> 
> Yes those would be a bit different as they would be physical memory
> accesses.  Those currently go through address_space_read in memory_dump
> (monitor/misc.c), and would have to use the MemoryDebugOps instead.
> That is the place to hook into in order to read the KVM page encryption
> bitmap (which is not per-CPU, so another MemoryDebugOps entry
> get_phys_addr_attrs?); the MemTxAttrs can then be passed to the read
> function in the MemoryDebugOps.
>

Actually, currently we do this in sev_dbg_crypt() in KVM itself by
checking the page encryption bitmap and if it is an un-encrypted guest
memory region then returning the un-encrypted buffer read from user
back to it as it is.  

> > This guest memory regions encryption status is found using KVM's page encryption bitmap
> > support which is part of the page encryption bitmap hypercall interface of the
> > KVM/QEMU SEV live migration patches.
> > 
> > As this additional debug support is dependent on the KVM's page encryption bitmap
> > support, are there any updates on KVM SEV live migration patches ?
> 
> Sorry about that, I've been busy with QEMU.  I'll review them as soon as
> possible.H

Looking forward to your updates and reviews regarding the same.

Thanks,
Ashish

Re: SEV guest debugging support for Qemu

[Paolo Bonzini]

On 26/09/20 01:48, Ashish Kalra wrote:
> Thanks for your input, i have one additional query with reference to this support :
> 
> For all explicitly unecrypted guest memory regions such as S/W IOTLB bounce buffers,
> dma_decrypted() allocated regions and for guest regions marked as "__bss_decrypted",
> we need to ensure that DBG_DECRYPT API calls are bypassed for such
> regions and those regions are dumped as un-encrypted.

Yes those would be a bit different as they would be physical memory
accesses.  Those currently go through address_space_read in memory_dump
(monitor/misc.c), and would have to use the MemoryDebugOps instead.
That is the place to hook into in order to read the KVM page encryption
bitmap (which is not per-CPU, so another MemoryDebugOps entry
get_phys_addr_attrs?); the MemTxAttrs can then be passed to the read
function in the MemoryDebugOps.

> This guest memory regions encryption status is found using KVM's page encryption bitmap
> support which is part of the page encryption bitmap hypercall interface of the
> KVM/QEMU SEV live migration patches.
> 
> As this additional debug support is dependent on the KVM's page encryption bitmap
> support, are there any updates on KVM SEV live migration patches ?

Sorry about that, I've been busy with QEMU.  I'll review them as soon as
possible.

Paolo

Re: SEV guest debugging support for Qemu

[Ashish Kalra]

Hello Paolo,

On Fri, Sep 25, 2020 at 10:56:10PM +0200, Paolo Bonzini wrote:
> On 25/09/20 22:46, Ashish Kalra wrote:
> > I was also considering abstracting this vendor/SEV specific debug
> > interface via the CPUClass object, the CPUClass object aleady has cpu
> > specific methods for doing things like guest VA to GPA translations like the
> > get_phys_page_attrs_debug() method and it will be a simple and clean
> > approach to override this method with a SEV specific
> > get_phys_page_attrs_debug() if SEV guest is active and SEV debug policy
> > is allowed. [...]
> > 
> > I can probably add new interfaces/methods to this CPUClass object for
> > guest memory read/writes for debugging purpose and then invoke the same
> > from the generic cpu_memory_rw_debug() interface. 
> > 
> > Let me know your thougts on abstracting this debug interface via the
> > CPUClass object ? 
> > 
> > Or the other option is to introduce the new MemoryDebugOps you described
> > above and additionally apply SEV/SEV-ES considerations in CPUClass
> > methods such as gdb_read_register, gdb_write_register, etc.
> 
> Yes, this makes the most sense, however you're right that you don't need
> translate in MemoryDebugOps.  I don't think read/write should be moved
> to CPUClass, however, since you can use a MemTxAttr to tell the
> read/write MemoryDebugOps whether the page is encrypted or not.
> 

Thanks for your input, i have one additional query with reference to this support :

For all explicitly unecrypted guest memory regions such as S/W IOTLB bounce buffers,
dma_decrypted() allocated regions and for guest regions marked as "__bss_decrypted",
we need to ensure that DBG_DECRYPT API calls are bypassed for such
regions and those regions are dumped as un-encrypted.

This guest memory regions encryption status is found using KVM's page encryption bitmap
support which is part of the page encryption bitmap hypercall interface of the
KVM/QEMU SEV live migration patches.

As this additional debug support is dependent on the KVM's page encryption bitmap
support, are there any updates on KVM SEV live migration patches ?

Thanks,
Ashish

Re: SEV guest debugging support for Qemu

[Paolo Bonzini]

On 25/09/20 22:46, Ashish Kalra wrote:
> I was also considering abstracting this vendor/SEV specific debug
> interface via the CPUClass object, the CPUClass object aleady has cpu
> specific methods for doing things like guest VA to GPA translations like the
> get_phys_page_attrs_debug() method and it will be a simple and clean
> approach to override this method with a SEV specific
> get_phys_page_attrs_debug() if SEV guest is active and SEV debug policy
> is allowed. [...]
> 
> I can probably add new interfaces/methods to this CPUClass object for
> guest memory read/writes for debugging purpose and then invoke the same
> from the generic cpu_memory_rw_debug() interface. 
> 
> Let me know your thougts on abstracting this debug interface via the
> CPUClass object ? 
> 
> Or the other option is to introduce the new MemoryDebugOps you described
> above and additionally apply SEV/SEV-ES considerations in CPUClass
> methods such as gdb_read_register, gdb_write_register, etc.

Yes, this makes the most sense, however you're right that you don't need
translate in MemoryDebugOps.  I don't think read/write should be moved
to CPUClass, however, since you can use a MemTxAttr to tell the
read/write MemoryDebugOps whether the page is encrypted or not.

Paolo

Re: SEV guest debugging support for Qemu

[Ashish Kalra]

Hello Paolo,

Thanks for your response.

On Fri, Sep 25, 2020 at 10:51:05AM +0200, Paolo Bonzini wrote:
> On 22/09/20 22:11, Ashish Kalra wrote:
> > 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.
> 
> My complaint was only about hooking into address_space_read and
> address_space_write; I think the hook should not touch general-purpose
> (non-debug) code if possible, so something like this:
> 
> typedef struct MemoryDebugOps {
>     hwaddr (*translate)(CPUState *cpu, target_ulong addr,
>                         MemTxAttrs *attrs);
>     MemTxResult (*read)(AddressSpace *as, hwaddr phys_addr,
>                         MemTxAttrs attrs, void *buf,
>                         hwaddr len);
>     MemTxResult (*write)(AddressSpace *as, hwaddr phys_addr,
>                          MemTxAttrs attrs, const void *buf,
>                          hwaddr len);
> } MemoryDebugOps;
> 
> These ops would be used only by cpu_memory_rw_debug and would default to
> 
> static const MemoryDebugOps default_debug_ops = {
>     .translate = cpu_get_phys_page_attrs_debug,
>     .read = address_space_read,
>     .write = address_space_write_rom
> };
> 
> static const MemoryDebugOps *debug_ops = &default_debug_ops;
> 

Yes, this looks like a good approach to proceed with.

I was also considering abstracting this vendor/SEV specific debug
interface via the CPUClass object, the CPUClass object aleady has cpu
specific methods for doing things like guest VA to GPA translations like the
get_phys_page_attrs_debug() method and it will be a simple and clean
approach to override this method with a SEV specific
get_phys_page_attrs_debug() if SEV guest is active and SEV debug policy
is allowed. This SEV specific method will then do guest page table walks
using the DBG_DECRYPT api and also clearing the c-bit bit on PxE copies.

One thought behind abstracting this vendor/SEV specific debug
interface via the CPUClass object is that the CPUClass object also has
methods for gdb register access such as
gdb_read_register()/gdb_write_register() which are invoked whenever
gdbstub does cpu register read/write. 

As part of this debug interface we also want to consider cpu register
access for SEV-ES, etc., for instance on SEV-ES we need to reject the
register access.  Again these gdb read/write methods in CPUClass object
can be overridden with SEV-ES specific variants which will then simply
return error when invoked. 

I can probably add new interfaces/methods to this CPUClass object for
guest memory read/writes for debugging purpose and then invoke the same
from the generic cpu_memory_rw_debug() interface. 

Let me know your thougts on abstracting this debug interface via the
CPUClass object ? 

Or the other option is to introduce the new MemoryDebugOps you described
above and additionally apply SEV/SEV-ES considerations in CPUClass
methods such as gdb_read_register, gdb_write_register, etc.

Thanks,
Ashish

Re: SEV guest debugging support for Qemu

[Paolo Bonzini]

On 22/09/20 22:11, Ashish Kalra wrote:
> 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.

My complaint was only about hooking into address_space_read and
address_space_write; I think the hook should not touch general-purpose
(non-debug) code if possible, so something like this:

typedef struct MemoryDebugOps {
    hwaddr (*translate)(CPUState *cpu, target_ulong addr,
                        MemTxAttrs *attrs);
    MemTxResult (*read)(AddressSpace *as, hwaddr phys_addr,
                        MemTxAttrs attrs, void *buf,
                        hwaddr len);
    MemTxResult (*write)(AddressSpace *as, hwaddr phys_addr,
                         MemTxAttrs attrs, const void *buf,
                         hwaddr len);
} MemoryDebugOps;

These ops would be used only by cpu_memory_rw_debug and would default to

static const MemoryDebugOps default_debug_ops = {
    .translate = cpu_get_phys_page_attrs_debug,
    .read = address_space_read,
    .write = address_space_write_rom
};

static const MemoryDebugOps *debug_ops = &default_debug_ops;

Paolo

Re: SEV guest debugging support for Qemu

[Dr. David Alan Gilbert]

* Ashish Kalra (ashish.kalra@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.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
> > 
> > 
> 
> 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&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
> > >>
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: SEV guest debugging support for Qemu

[Ashish Kalra]

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

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.

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

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

Re: SEV guest debugging support for Qemu

[Ashish Kalra]

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
> 

Re: SEV guest debugging support for Qemu

[Dr. David Alan Gilbert]

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

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

Dave

> 5. many more checks
> 
> Looking fwd. to your feedback/comments on the above approach or other any other suggestions.
> 
> Thanks,
> Ashish
> 
> [1] -> http://next.patchew.org/QEMU/20180308124901.83533-1-brijesh.singh@amd.com/20180308124901.83533-29-brijesh.singh@amd.com/
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

SEV guest debugging support for Qemu

[Ashish Kalra]

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.

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.

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.

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.

5. many more checks

Looking fwd. to your feedback/comments on the above approach or other any other suggestions.

Thanks,
Ashish

[1] -> http://next.patchew.org/QEMU/20180308124901.83533-1-brijesh.singh@amd.com/20180308124901.83533-29-brijesh.singh@amd.com/