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

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

[Andrew Cooper]

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

[Roger Pau Monné]

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

[Ian Jackson]

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>