| Message ID | 148900628810.27090.3461280348833651824.stgit@brijesh-build-machine (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 08, 2017 at 03:51:28PM -0500, Brijesh Singh wrote: > Add a new debug attribute, the attribute should be set when guest memory > accesses are performed for debug purposes. > The attribute will be used in SEV guest, where we need to distinguish normal > vs debug access to guest memory. In debug mode, we need to use SEV commands > to access the guest memory. > > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > --- > include/exec/memattrs.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h > index e601061..b802073 100644 > --- a/include/exec/memattrs.h > +++ b/include/exec/memattrs.h > @@ -37,6 +37,8 @@ typedef struct MemTxAttrs { > unsigned int user:1; > /* Requester ID (for MSI for example) */ > unsigned int requester_id:16; > + /* Memory access for debug purposes */ What does "debug purposes" mean? gdbstub? Can the guest initiate debug memory accesses or is the purely QEMU-internal? > + unsigned int debug:1; > } MemTxAttrs; > > /* Bus masters which don't specify any attributes will get this, > @@ -46,4 +48,6 @@ typedef struct MemTxAttrs { > */ > #define MEMTXATTRS_UNSPECIFIED ((MemTxAttrs) { .unspecified = 1 }) > > +/* Access the guest memory for debug purposes */ > +#define MEMTXATTRS_DEBUG ((MemTxAttrs) { .debug = 1 }) > #endif > >
Hi Stefan, On 03/23/2017 06:29 AM, Stefan Hajnoczi wrote: > On Wed, Mar 08, 2017 at 03:51:28PM -0500, Brijesh Singh wrote: >> Add a new debug attribute, the attribute should be set when guest memory >> accesses are performed for debug purposes. >> The attribute will be used in SEV guest, where we need to distinguish normal >> vs debug access to guest memory. In debug mode, we need to use SEV commands >> to access the guest memory. >> >> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> >> --- >> include/exec/memattrs.h | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h >> index e601061..b802073 100644 >> --- a/include/exec/memattrs.h >> +++ b/include/exec/memattrs.h >> @@ -37,6 +37,8 @@ typedef struct MemTxAttrs { >> unsigned int user:1; >> /* Requester ID (for MSI for example) */ >> unsigned int requester_id:16; >> + /* Memory access for debug purposes */ > > What does "debug purposes" mean? gdbstub? Can the guest initiate debug > memory accesses or is the purely QEMU-internal? > What I mean by that is, any access to the guest memory within Qemu internal functions (e.g gdbstub, qemu monitor's memory dump, info mem, info tlb etc). I have also introduced debug version of ldl_phys, ldq_phys, cpu_physical_memory_* [1] and have updated hmp monitor code to use the debug version api's when accessing the guest memory [2]. [1] http://marc.info/?l=qemu-devel&m=148900832814697&w=2 [2] http://marc.info/?l=qemu-devel&m=148900831414693&w=2 Note: SEV debug API's are meant to be used by hypervisor to decrypt/encrypt guest memory. But when you are inside the guest, the guest will have access to decrypted data and does not need to call down to hypervisor for debug access. >> + unsigned int debug:1; >> } MemTxAttrs; >> >> /* Bus masters which don't specify any attributes will get this, >> @@ -46,4 +48,6 @@ typedef struct MemTxAttrs { >> */ >> #define MEMTXATTRS_UNSPECIFIED ((MemTxAttrs) { .unspecified = 1 }) >> >> +/* Access the guest memory for debug purposes */ >> +#define MEMTXATTRS_DEBUG ((MemTxAttrs) { .debug = 1 }) >> #endif >> >>
On Thu, Mar 23, 2017 at 01:14:17PM -0500, Brijesh Singh wrote: > Hi Stefan, > > > On 03/23/2017 06:29 AM, Stefan Hajnoczi wrote: > > On Wed, Mar 08, 2017 at 03:51:28PM -0500, Brijesh Singh wrote: > > > Add a new debug attribute, the attribute should be set when guest memory > > > accesses are performed for debug purposes. > > > The attribute will be used in SEV guest, where we need to distinguish normal > > > vs debug access to guest memory. In debug mode, we need to use SEV commands > > > to access the guest memory. > > > > > > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > > > --- > > > include/exec/memattrs.h | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h > > > index e601061..b802073 100644 > > > --- a/include/exec/memattrs.h > > > +++ b/include/exec/memattrs.h > > > @@ -37,6 +37,8 @@ typedef struct MemTxAttrs { > > > unsigned int user:1; > > > /* Requester ID (for MSI for example) */ > > > unsigned int requester_id:16; > > > + /* Memory access for debug purposes */ > > > > What does "debug purposes" mean? gdbstub? Can the guest initiate debug > > memory accesses or is the purely QEMU-internal? > > > > What I mean by that is, any access to the guest memory within Qemu internal functions > (e.g gdbstub, qemu monitor's memory dump, info mem, info tlb etc). I have also introduced > debug version of ldl_phys, ldq_phys, cpu_physical_memory_* [1] and have updated hmp monitor > code to use the debug version api's when accessing the guest memory [2]. > > [1] http://marc.info/?l=qemu-devel&m=148900832814697&w=2 > [2] http://marc.info/?l=qemu-devel&m=148900831414693&w=2 > > > Note: SEV debug API's are meant to be used by hypervisor to decrypt/encrypt guest memory. > But when you are inside the guest, the guest will have access to decrypted data and does > not need to call down to hypervisor for debug access. People reading the code in memattrs.h aren't thinking about AMD SEV, so "debug purposes" isn't meaningful in this context. I suggest something like this instead: /* Debug memory access for AMD SEV */ That way it's clear this "debug" flag has a very specific meaning in the context of memory encryption. Stefan
On 03/24/2017 10:36 AM, Stefan Hajnoczi wrote: > > I suggest something like this instead: > > /* Debug memory access for AMD SEV */ > > That way it's clear this "debug" flag has a very specific meaning in the > context of memory encryption. > Sure, will update comments. -Brijesh
diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h index e601061..b802073 100644 --- a/include/exec/memattrs.h +++ b/include/exec/memattrs.h @@ -37,6 +37,8 @@ typedef struct MemTxAttrs { unsigned int user:1; /* Requester ID (for MSI for example) */ unsigned int requester_id:16; + /* Memory access for debug purposes */ + unsigned int debug:1; } MemTxAttrs; /* Bus masters which don't specify any attributes will get this, @@ -46,4 +48,6 @@ typedef struct MemTxAttrs { */ #define MEMTXATTRS_UNSPECIFIED ((MemTxAttrs) { .unspecified = 1 }) +/* Access the guest memory for debug purposes */ +#define MEMTXATTRS_DEBUG ((MemTxAttrs) { .debug = 1 }) #endif
Add a new debug attribute, the attribute should be set when guest memory accesses are performed for debug purposes. The attribute will be used in SEV guest, where we need to distinguish normal vs debug access to guest memory. In debug mode, we need to use SEV commands to access the guest memory. Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- include/exec/memattrs.h | 4 ++++ 1 file changed, 4 insertions(+)