Re: [PATCH v4 02/10] x86/vmx: add IPT cpu feature

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 01/07/2020 17:18, Julien Grall wrote:
>
>
> On 01/07/2020 17:17, Julien Grall wrote:
>>
>>
>> On 01/07/2020 17:06, Andrew Cooper wrote:
>>> On 01/07/2020 16:12, Julien Grall wrote:
>>>> On 30/06/2020 13:33, Michał Leszczyński wrote:
>>>>> @@ -305,7 +311,6 @@ static int vmx_init_vmcs_config(void)
>>>>>                   SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS |
>>>>>                   SECONDARY_EXEC_XSAVES |
>>>>>                   SECONDARY_EXEC_TSC_SCALING);
>>>>> -        rdmsrl(MSR_IA32_VMX_MISC, _vmx_misc_cap);
>>>>>            if ( _vmx_misc_cap & VMX_MISC_VMWRITE_ALL )
>>>>>                opt |= SECONDARY_EXEC_ENABLE_VMCS_SHADOWING;
>>>>>            if ( opt_vpid_enabled )
>>>>> diff --git a/xen/common/domain.c b/xen/common/domain.c
>>>>> index 7cc9526139..0a33e0dfd6 100644
>>>>> --- a/xen/common/domain.c
>>>>> +++ b/xen/common/domain.c
>>>>> @@ -82,6 +82,8 @@ struct vcpu *idle_vcpu[NR_CPUS] __read_mostly;
>>>>>      vcpu_info_t dummy_vcpu_info;
>>>>>    +bool_t vmtrace_supported;
>>>>
>>>> All the code looks x86 specific. So may I ask why this was implemented
>>>> in common code?
>>>
>>> There were some questions directed specifically at the ARM maintainers
>>> about CoreSight, which have gone unanswered.
>>
>> I can only find one question related to the size. Is there any other?
>>
>> I don't know how the interface will look like given that AFAICT the
>> buffer may be embedded in the HW. We would need to investigate how to
>> differentiate between two domUs in this case without impacting the
>> performance in the common code.
>
> s/in the common code/during the context switch/
>
>> So I think it is a little premature to implement this in common code
>> and always compiled in for Arm. It would be best if this stay in x86
>> code.

I've just checked with a colleague.  CoreSight can dump to a memory
buffer - there's even a decode library for the packet stream
https://github.com/Linaro/OpenCSD, although ultimately it is platform
specific as to whether the feature is supported.

Furthermore, the choice isn't "x86 vs ARM", now that RISCv support is
on-list, and Power9 is floating on the horizon.

For the sake of what is literally just one byte in common code, I stand
my original suggestion of this being a common interface.  It is not
something which should be x86 specific.

~Andrew