[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

On 01/07/2020 19:06, Andrew Cooper wrote:
On 01/07/2020 19:02, Julien Grall wrote:

On 01/07/2020 18:26, Andrew Cooper wrote:
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_XSAVES |
-        rdmsrl(MSR_IA32_VMX_MISC, _vmx_misc_cap);
             if ( _vmx_misc_cap & VMX_MISC_VMWRITE_ALL )
             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
in common code?

There were some questions directed specifically at the ARM
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

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.

This argument can also be used against putting in common code. What I
am the most concern of is we are trying to guess how the interface
will look like for another architecture. Your suggested interface may
work, but this also may end up to be a complete mess.

So I think we want to wait for a new architecture to use vmtrace
before moving to common code. This is not going to be a massive effort
to move that bit in common if needed.

I suggest you read the series.

Already went through the series and ...

The only thing in common code is the bit of the interface saying "I'd
like buffers this big please".

... I stand by my point. There is no need to have this code in common code until someone else need it. This code can be easily implemented in arch_domain_create().


Julien Grall



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.