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

2021-03-04 Thread 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 
> ---
> CC: Jan Beulich 
> CC: Roger Pau Monné 
> CC: Wei Liu 
> CC: Stefano Stabellini 
> CC: Julien Grall 
> CC: Volodymyr Babchuk 
> CC: Ian Jackson 
> 
> 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 



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

2021-03-04 Thread Roger Pau Monné
On Thu, Mar 04, 2021 at 10:48:05AM +, 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 

Reviewed-by: Roger Pau Monné 

Thanks, Roger.



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

2021-03-04 Thread 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 
---
CC: Jan Beulich 
CC: Roger Pau Monné 
CC: Wei Liu 
CC: Stefano Stabellini 
CC: Julien Grall 
CC: Volodymyr Babchuk 
CC: Ian Jackson 

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 = _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 = _vcpus;
 
 data->vcpus = d->max_vcpus;
+const_op = false;
 rc = 0;
 break;
 }
-- 
2.11.0