Re: [Xen-devel] [PATCH v2 1/5] x86/hvm: Handle viridian MSRs via the new guest_{rd, wr}msr() infrastructure

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.03.18 at 16:47, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 13/03/18 15:20, Jan Beulich wrote:
>>>>> On 07.03.18 at 19:58, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> @@ -175,11 +177,26 @@ int guest_rdmsr(const struct vcpu *v, uint32_t msr, 
> uint64_t *val)
>>>                 _MSR_MISC_FEATURES_CPUID_FAULTING;
>>>          break;
>>>  
>>> +    case MSR_HYPERVISOR_START ... MSR_HYPERVISOR_START + NR_VIRIDIAN_MSRS 
>>> - 
> 
>>> 1:
>>> +        if ( is_viridian_domain(d) )
>>> +        {
>>> +            ret = guest_rdmsr_viridian(v, msr, val);
>>> +            goto out;
>>> +        }
>>> +
>>> +        /* Fallthrough. */
>>>      default:
>>>          return X86EMUL_UNHANDLEABLE;
>>>      }
>>>  
>>> -    return X86EMUL_OKAY;
>>> + out:
>> I've noticed this only in the context of patch 4, but why is this label
>> and yet another unnecessary "goto" here? That "goto" could simply
>> be "break" afaics.
> 
> Ah - that is for changes which I haven't posted yet.
> 
> When we get onto MSRs which might be in the load/save lists, or may be
> stashed in the VMCB/VMCS rather than in real hardware, we need to call
> back into arch specific code when an update is completed.

But I don't think this requires a goto here, does it? If that future
code structure really can't get away without, so be it (then). But
please let's evaluate that at the time you have that code ready.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel