[PATCH for-4.15] xen/dmop: Fix XEN_DMOP_nr_vcpus to actually return data

Andrew Cooper posted 1 patch 3 years, 1 month ago
Test gitlab-ci passed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20210304104805.27601-1-andrew.cooper3@citrix.com
xen/arch/arm/dm.c     | 1 +
xen/arch/x86/hvm/dm.c | 1 +
2 files changed, 2 insertions(+)
[PATCH for-4.15] xen/dmop: Fix XEN_DMOP_nr_vcpus to actually return data
Posted by Andrew Cooper 3 years, 1 month ago
The const_op boolean needs clobbering to cause data to be written back to the
caller.

Fixes: c4441ab1f1 ("dmop: Add XEN_DMOP_nr_vcpus")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Ian Jackson <iwj@xenproject.org>

If we weren't in a release freeze, I'd rewrite large chunks of this.
'const_op' is what we call 'copyback' everywhere else, but with inverted
sense.  I'll guess this gets added to the pile of other unbreakage work which
might happen in 4.16

My ad-hoc unit test appears to have had a false positive for the success case,
which I've fixed.  However, the chances of the full test landing in 4.15 is
getting slimmer, not to mention the fact that it curretly takes out Xen with
reference counting error...

As for 4.15, this is a bug in a brand-newly introduced hypercall, and is of 0
risk for other areas of the release.  If this bugfix is not taken, we should
revert c4441ab1f1 to take the hypercall out, but this would be a bad move.
---
 xen/arch/arm/dm.c     | 1 +
 xen/arch/x86/hvm/dm.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/xen/arch/arm/dm.c b/xen/arch/arm/dm.c
index d689e336fd..1b3fd6bc7d 100644
--- a/xen/arch/arm/dm.c
+++ b/xen/arch/arm/dm.c
@@ -128,6 +128,7 @@ int dm_op(const struct dmop_args *op_args)
         struct xen_dm_op_nr_vcpus *data = &op.u.nr_vcpus;
 
         data->vcpus = d->max_vcpus;
+        const_op = false;
         rc = 0;
         break;
     }
diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
index f4f0910463..b60b9f3364 100644
--- a/xen/arch/x86/hvm/dm.c
+++ b/xen/arch/x86/hvm/dm.c
@@ -612,6 +612,7 @@ int dm_op(const struct dmop_args *op_args)
         struct xen_dm_op_nr_vcpus *data = &op.u.nr_vcpus;
 
         data->vcpus = d->max_vcpus;
+        const_op = false;
         rc = 0;
         break;
     }
-- 
2.11.0


Re: [PATCH for-4.15] xen/dmop: Fix XEN_DMOP_nr_vcpus to actually return data
Posted by Roger Pau Monné 3 years, 1 month ago
On Thu, Mar 04, 2021 at 10:48:05AM +0000, Andrew Cooper wrote:
> The const_op boolean needs clobbering to cause data to be written back to the
> caller.
> 
> Fixes: c4441ab1f1 ("dmop: Add XEN_DMOP_nr_vcpus")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

Re: [PATCH for-4.15] xen/dmop: Fix XEN_DMOP_nr_vcpus to actually return data
Posted by Ian Jackson 3 years, 1 month ago
Andrew Cooper writes ("[PATCH for-4.15] xen/dmop: Fix XEN_DMOP_nr_vcpus to actually return data"):
> The const_op boolean needs clobbering to cause data to be written back to the
> caller.
> 
> Fixes: c4441ab1f1 ("dmop: Add XEN_DMOP_nr_vcpus")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Wei Liu <wl@xen.org>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien@xen.org>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> CC: Ian Jackson <iwj@xenproject.org>
> 
> If we weren't in a release freeze, I'd rewrite large chunks of this.
> 'const_op' is what we call 'copyback' everywhere else, but with inverted
> sense.  I'll guess this gets added to the pile of other unbreakage work which
> might happen in 4.16
> 
> My ad-hoc unit test appears to have had a false positive for the success case,
> which I've fixed.  However, the chances of the full test landing in 4.15 is
> getting slimmer, not to mention the fact that it curretly takes out Xen with
> reference counting error...
> 
> As for 4.15, this is a bug in a brand-newly introduced hypercall, and is of 0
> risk for other areas of the release.  If this bugfix is not taken, we should
> revert c4441ab1f1 to take the hypercall out, but this would be a bad move.

Thanks for that explanation.

Release-Acked-by: Ian Jackson <iwj@xenproject.org>