| Message ID | 20211115121741.3719-4-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | gnttab: fallout/improvements from max version | expand |
On 15.11.2021 13:17, Roger Pau Monne wrote: > Set the grant table version for the created domains to use version 1, > as such tests domains don't require the usage of the grant table at > all. A TODO note is added to switch those dummy domains to not have a > grant table at all when possible. Without setting the grant version > the domains for the tests cannot be created. > > Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain') > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > Reported-by: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
On 15/11/2021 12:17, Roger Pau Monne wrote: > Set the grant table version for the created domains to use version 1, > as such tests domains don't require the usage of the grant table at > all. A TODO note is added to switch those dummy domains to not have a > grant table at all when possible. Without setting the grant version > the domains for the tests cannot be created. > > Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain') > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > Reported-by: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > Cc: Ian Jackson <iwj@xenproject.org> > > This patch only modifies a test, so it should be safe to commit as > it's not going to cause any changes to the hypervisor or the tools. > Worse that could happen is it makes the test even more broken, but > it's already unusable. What do you mean unusable? Other than this, the test works. > --- > tools/tests/tsx/test-tsx.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/tools/tests/tsx/test-tsx.c b/tools/tests/tsx/test-tsx.c > index fab99c135e..f1dcff4c30 100644 > --- a/tools/tests/tsx/test-tsx.c > +++ b/tools/tests/tsx/test-tsx.c > @@ -444,6 +444,8 @@ static void test_guests(void) > struct xen_domctl_createdomain c = { > .max_vcpus = 1, > .max_grant_frames = 1, > + /* TODO: switch to 0 once support for no grant table is added. */ I'd avoid these TODOs. It's test code, so really doesn't matter too much. However, there is a further task for this test to actually boot enough of a guest to dump CPUID as visible inside the guest, and cross-check with the toolstack's view of the data. How exactly to get that data out of the VM is an open question, but it might involve xenconsoled. ~Andrew
On Mon, Nov 15, 2021 at 12:43:34PM +0000, Andrew Cooper wrote: > On 15/11/2021 12:17, Roger Pau Monne wrote: > > Set the grant table version for the created domains to use version 1, > > as such tests domains don't require the usage of the grant table at > > all. A TODO note is added to switch those dummy domains to not have a > > grant table at all when possible. Without setting the grant version > > the domains for the tests cannot be created. > > > > Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain') > > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > > Reported-by: Jan Beulich <jbeulich@suse.com> > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > --- > > Cc: Ian Jackson <iwj@xenproject.org> > > > > This patch only modifies a test, so it should be safe to commit as > > it's not going to cause any changes to the hypervisor or the tools. > > Worse that could happen is it makes the test even more broken, but > > it's already unusable. > > What do you mean unusable? Other than this, the test works. I mean, it's unusable because I broke it with the gnttab change, and that's it's current status unless this patch is applied. > > --- > > tools/tests/tsx/test-tsx.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/tools/tests/tsx/test-tsx.c b/tools/tests/tsx/test-tsx.c > > index fab99c135e..f1dcff4c30 100644 > > --- a/tools/tests/tsx/test-tsx.c > > +++ b/tools/tests/tsx/test-tsx.c > > @@ -444,6 +444,8 @@ static void test_guests(void) > > struct xen_domctl_createdomain c = { > > .max_vcpus = 1, > > .max_grant_frames = 1, > > + /* TODO: switch to 0 once support for no grant table is added. */ > > I'd avoid these TODOs. It's test code, so really doesn't matter too much. I'm fine with this, when looking at the test code I didn't see any reason why gnttab was needed, so I thought it would be a fine use-case for opting out to grant table. As you say it's a test case, so it doesn't matter much. Do you want me to repost with the comments removed? Thanks, Roger.
On 15/11/2021 13:58, Roger Pau Monné wrote: > On Mon, Nov 15, 2021 at 12:43:34PM +0000, Andrew Cooper wrote: >> On 15/11/2021 12:17, Roger Pau Monne wrote: >>> Set the grant table version for the created domains to use version 1, >>> as such tests domains don't require the usage of the grant table at >>> all. A TODO note is added to switch those dummy domains to not have a >>> grant table at all when possible. Without setting the grant version >>> the domains for the tests cannot be created. >>> >>> Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain') >>> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> >>> Reported-by: Jan Beulich <jbeulich@suse.com> >>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >>> --- >>> Cc: Ian Jackson <iwj@xenproject.org> >>> >>> This patch only modifies a test, so it should be safe to commit as >>> it's not going to cause any changes to the hypervisor or the tools. >>> Worse that could happen is it makes the test even more broken, but >>> it's already unusable. >> What do you mean unusable? Other than this, the test works. > I mean, it's unusable because I broke it with the gnttab change, and > that's it's current status unless this patch is applied. > >>> --- >>> tools/tests/tsx/test-tsx.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/tools/tests/tsx/test-tsx.c b/tools/tests/tsx/test-tsx.c >>> index fab99c135e..f1dcff4c30 100644 >>> --- a/tools/tests/tsx/test-tsx.c >>> +++ b/tools/tests/tsx/test-tsx.c >>> @@ -444,6 +444,8 @@ static void test_guests(void) >>> struct xen_domctl_createdomain c = { >>> .max_vcpus = 1, >>> .max_grant_frames = 1, >>> + /* TODO: switch to 0 once support for no grant table is added. */ >> I'd avoid these TODOs. It's test code, so really doesn't matter too much. > I'm fine with this, when looking at the test code I didn't see any > reason why gnttab was needed, so I thought it would be a fine use-case > for opting out to grant table. As you say it's a test case, so it > doesn't matter much. > > Do you want me to repost with the comments removed? I'm happy to fix on commit, if nothing else needs reposting. ~Andrew
Roger Pau Monne writes ("[PATCH for-4.16 3/4] test/tsx: set grant version for created domains"):
> Set the grant table version for the created domains to use version 1,
> as such tests domains don't require the usage of the grant table at
> all. A TODO note is added to switch those dummy domains to not have a
> grant table at all when possible. Without setting the grant version
> the domains for the tests cannot be created.
>
> Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain')
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reported-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
diff --git a/tools/tests/tsx/test-tsx.c b/tools/tests/tsx/test-tsx.c index fab99c135e..f1dcff4c30 100644 --- a/tools/tests/tsx/test-tsx.c +++ b/tools/tests/tsx/test-tsx.c @@ -444,6 +444,8 @@ static void test_guests(void) struct xen_domctl_createdomain c = { .max_vcpus = 1, .max_grant_frames = 1, + /* TODO: switch to 0 once support for no grant table is added. */ + .grant_opts = XEN_DOMCTL_GRANT_version(1), }; printf("Testing PV guest\n"); @@ -456,6 +458,8 @@ static void test_guests(void) .flags = XEN_DOMCTL_CDF_hvm, .max_vcpus = 1, .max_grant_frames = 1, + /* TODO: switch to 0 once support for no grant table is added. */ + .grant_opts = XEN_DOMCTL_GRANT_version(1), .arch = { .emulation_flags = XEN_X86_EMU_LAPIC, },
Set the grant table version for the created domains to use version 1, as such tests domains don't require the usage of the grant table at all. A TODO note is added to switch those dummy domains to not have a grant table at all when possible. Without setting the grant version the domains for the tests cannot be created. Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain') Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Reported-by: Jan Beulich <jbeulich@suse.com> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Ian Jackson <iwj@xenproject.org> This patch only modifies a test, so it should be safe to commit as it's not going to cause any changes to the hypervisor or the tools. Worse that could happen is it makes the test even more broken, but it's already unusable. --- tools/tests/tsx/test-tsx.c | 4 ++++ 1 file changed, 4 insertions(+)