I've noticed the bugs fixed in patch 1 only while doing the other cleanup.

1: fix error code use
2: drop dead variable updates
3: fold duplicate vpset retrieval code

Jan

On 18/11/2021 13:12, Jan Beulich wrote:
> I've noticed the bugs fixed in patch 1 only while doing the other cleanup.
>
> 1: fix error code use
> 2: drop dead variable updates
> 3: fold duplicate vpset retrieval code

Oh, nice.  This makes it rather easier to do the flush short-circuit for
HV_GENERIC_SET_ALL.

I'll try importing this patchset onto my branch and having a go at it.

~Andrew

On 18.11.2021 14:20, Andrew Cooper wrote:
> On 18/11/2021 13:12, Jan Beulich wrote:
>> I've noticed the bugs fixed in patch 1 only while doing the other cleanup.
>>
>> 1: fix error code use
>> 2: drop dead variable updates
>> 3: fold duplicate vpset retrieval code
> 
> Oh, nice.  This makes it rather easier to do the flush short-circuit for
> HV_GENERIC_SET_ALL.

To be honest I first thought it might, but now I'm not sure anymore.

Jan

On 18/11/2021 13:34, Jan Beulich wrote:
> On 18.11.2021 14:20, Andrew Cooper wrote:
>> On 18/11/2021 13:12, Jan Beulich wrote:
>>> I've noticed the bugs fixed in patch 1 only while doing the other cleanup.
>>>
>>> 1: fix error code use
>>> 2: drop dead variable updates
>>> 3: fold duplicate vpset retrieval code
>> Oh, nice.  This makes it rather easier to do the flush short-circuit for
>> HV_GENERIC_SET_ALL.
> To be honest I first thought it might, but now I'm not sure anymore.

Just this delta:

diff --git a/xen/arch/x86/hvm/viridian/viridian.c
b/xen/arch/x86/hvm/viridian/viridian.c
index 658e68f7f2bb..c8c05bfb04a1 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -771,7 +771,8 @@ static int hvcall_flush_ex(const union
hypercall_input *input,
                                   sizeof(input_params)) != HVMTRANS_okay )
         return -EINVAL;
 
-    if ( input_params.flags & HV_FLUSH_ALL_PROCESSORS )
+    if ( input_params.flags & HV_FLUSH_ALL_PROCESSORS ||
+         input_params.set.format == HV_GENERIC_SET_ALL )
         vcpu_bitmap = NULL;
     else
     {

which now I come to think of it independent of your cleanup, and small
enough to be folded into my main IPI change.

~Andrew

On 18.11.2021 19:05, Andrew Cooper wrote:
> On 18/11/2021 13:34, Jan Beulich wrote:
>> On 18.11.2021 14:20, Andrew Cooper wrote:
>>> On 18/11/2021 13:12, Jan Beulich wrote:
>>>> I've noticed the bugs fixed in patch 1 only while doing the other cleanup.
>>>>
>>>> 1: fix error code use
>>>> 2: drop dead variable updates
>>>> 3: fold duplicate vpset retrieval code
>>> Oh, nice.  This makes it rather easier to do the flush short-circuit for
>>> HV_GENERIC_SET_ALL.
>> To be honest I first thought it might, but now I'm not sure anymore.
> 
> Just this delta:
> 
> diff --git a/xen/arch/x86/hvm/viridian/viridian.c
> b/xen/arch/x86/hvm/viridian/viridian.c
> index 658e68f7f2bb..c8c05bfb04a1 100644
> --- a/xen/arch/x86/hvm/viridian/viridian.c
> +++ b/xen/arch/x86/hvm/viridian/viridian.c
> @@ -771,7 +771,8 @@ static int hvcall_flush_ex(const union
> hypercall_input *input,
>                                    sizeof(input_params)) != HVMTRANS_okay )
>          return -EINVAL;
>  
> -    if ( input_params.flags & HV_FLUSH_ALL_PROCESSORS )
> +    if ( input_params.flags & HV_FLUSH_ALL_PROCESSORS ||
> +         input_params.set.format == HV_GENERIC_SET_ALL )
>          vcpu_bitmap = NULL;
>      else
>      {
> 
> which now I come to think of it independent of your cleanup, and small
> enough to be folded into my main IPI change.

FTAOD please keep my R-b there with this addition.

Jan