|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 1/4] xen/libxc: Allow changes to hypervisor CPUID leaf from config file
On 11/03/14 14:07, Boris Ostrovsky wrote:
> On 03/11/2014 06:10 AM, Andrew Cooper wrote:
>> On 11/03/14 03:54, Boris Ostrovsky wrote:
>>> Currently only "real" cpuid leaves can be overwritten by users via
>>> 'cpuid' option in the configuration file. This patch provides
>>> ability to
>>> do the same for hypervisor leaves (those in the 0x40000000 range).
>>>
>>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
>> How? There is nothing stopping leaves in 0x40000000 being set in the
>> policy with XEN_DOMCTL_set_cpuid, but I dont see anything which plumbs
>> this together at the Xen level.
>
> Right. What this patch mostly provides is ability to query the
> hypervisor (via sysctl) about default values of hypervisor CPUID leaf
> from userspace. We cannot use CPUID instruction here (for dom0). And
> /dev/cpu/<n>/cpuid may not exist.
The XEN_FORCED_EMULATION prefix would be fine, and not require a new
custom hypercall, but only an HVM guest is going to care whether it can
find this magic leaf.
>
> We then use these values plus whatever the user requested (because the
> user may ask for only one of the 4 registers) to pass to the
> subsequent XEN_DOMCTL_set_cpuid call.
I currently have a project to fix this braindead thinking of having Xen
and libxc guess at what eachother supports and will report.
>
>>
>>> ---
>>> tools/libxc/xc_cpuid_x86.c | 23 ++++++++++++++++++++++-
>>> tools/libxc/xc_misc.c | 18 ++++++++++++++++++
>>> tools/libxc/xenctrl.h | 2 ++
>>> xen/arch/x86/domain.c | 19 ++++++++++++++++---
>>> xen/arch/x86/sysctl.c | 17 +++++++++++++++++
>>> xen/arch/x86/traps.c | 3 +++
>>> xen/include/asm-x86/domain.h | 7 +++++++
>>> xen/include/public/sysctl.h | 18 ++++++++++++++++++
>>> 8 files changed, 103 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
>>> index bbbf9b8..544a0fd 100644
>>> --- a/tools/libxc/xc_cpuid_x86.c
>>> +++ b/tools/libxc/xc_cpuid_x86.c
>>> @@ -33,6 +33,8 @@
>>> #define DEF_MAX_INTELEXT 0x80000008u
>>> #define DEF_MAX_AMDEXT 0x8000001cu
>>> +#define HYPERVISOR_LEAF(idx) (((idx) & 0x40000000) == 0x40000000)
>>> +
>> This check is wrong.
>
> Because of Viridian leaves? Or something else?
It should be (((idx) & 0xf0000000) == 0x40000000)
According to the AMD and Intel manuals, it is strictly leaf 0x40000000
reserved for hypervisor use, not 0xC0000000 or others.
>
>>
>>> static int hypervisor_is_64bit(xc_interface *xch)
>>> {
>>> xen_capabilities_info_t xen_caps = "";
>>> @@ -555,6 +557,9 @@ static int xc_cpuid_policy(
>>> {
>>> xc_dominfo_t info;
>>> + if ( HYPERVISOR_LEAF(input[0]) )
>>> + return 0;
>>> +
>>> if ( xc_domain_getinfo(xch, domid, 1, &info) == 0 )
>>> return -EINVAL;
>>> @@ -754,7 +759,23 @@ int xc_cpuid_set(
>>> memset(config_transformed, 0, 4 * sizeof(*config_transformed));
>>> - cpuid(input, regs);
>>> + if ( HYPERVISOR_LEAF(input[0]) )
>>> + {
>>> + xc_cpuid_t cpuid;
>>> +
>>> + cpuid.input[0] = input[0];
>>> + cpuid.input[1] = input[1];
>>> +
>>> + if (xc_cpuid(xch, &cpuid))
>>> + regs[0] = regs[1] = regs[2] = regs[3] = 0;
>>> + else {
>>> + regs[0] = cpuid.eax;
>>> + regs[1] = cpuid.ebx;
>>> + regs[2] = cpuid.ecx;
>>> + regs[3] = cpuid.edx;
>>> + }
>>> + } else
>>> + cpuid(input, regs);
>>> memcpy(polregs, regs, sizeof(regs));
>>> xc_cpuid_policy(xch, domid, input, polregs);
>>> diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
>>> index 3303454..4e8669b 100644
>>> --- a/tools/libxc/xc_misc.c
>>> +++ b/tools/libxc/xc_misc.c
>>> @@ -159,6 +159,24 @@ int xc_send_debug_keys(xc_interface *xch, char
>>> *keys)
>>> return ret;
>>> }
>>> +int xc_cpuid(xc_interface *xch,
>>> + xc_cpuid_t *cpuid)
>>> +{
>>> + int ret;
>>> + DECLARE_SYSCTL;
>>> +
>>> + sysctl.cmd = XEN_SYSCTL_cpuid_op;
>>> +
>>> + memcpy(&sysctl.u.cpuid, cpuid, sizeof(*cpuid));
>>> +
>>> + if ( (ret = do_sysctl(xch, &sysctl)) != 0 )
>>> + return ret;
>>> +
>>> + memcpy(cpuid, &sysctl.u.cpuid, sizeof(*cpuid));
>>> +
>>> + return 0;
>>> +}
>>> +
>>> int xc_physinfo(xc_interface *xch,
>>> xc_physinfo_t *put_info)
>>> {
>>> diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
>>> index 13f816b..6c709f5 100644
>>> --- a/tools/libxc/xenctrl.h
>>> +++ b/tools/libxc/xenctrl.h
>>> @@ -1135,6 +1135,7 @@ int xc_send_debug_keys(xc_interface *xch, char
>>> *keys);
>>> typedef xen_sysctl_physinfo_t xc_physinfo_t;
>>> typedef xen_sysctl_topologyinfo_t xc_topologyinfo_t;
>>> typedef xen_sysctl_numainfo_t xc_numainfo_t;
>>> +typedef xen_sysctl_cpuid_t xc_cpuid_t;
>>> typedef uint32_t xc_cpu_to_node_t;
>>> typedef uint32_t xc_cpu_to_socket_t;
>>> @@ -1146,6 +1147,7 @@ typedef uint32_t xc_node_to_node_dist_t;
>>> int xc_physinfo(xc_interface *xch, xc_physinfo_t *info);
>>> int xc_topologyinfo(xc_interface *xch, xc_topologyinfo_t *info);
>>> int xc_numainfo(xc_interface *xch, xc_numainfo_t *info);
>>> +int xc_cpuid(xc_interface *xch, xc_cpuid_t *cpuid);
>>> int xc_sched_id(xc_interface *xch,
>>> int *sched_id);
>>> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
>>> index c42a079..98e2b5f 100644
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -1997,7 +1997,7 @@ void arch_dump_vcpu_info(struct vcpu *v)
>>> vpmu_dump(v);
>>> }
>>> -void domain_cpuid(
>>> +bool_t domain_cpuid_exists(
>>> struct domain *d,
>>> unsigned int input,
>>> unsigned int sub_input,
>>> @@ -2030,11 +2030,24 @@ void domain_cpuid(
>>> !d->disable_migrate && !d->arch.vtsc )
>>> *edx &= ~(1u<<8); /* TSC Invariant */
>>> - return;
>>> + return 1;
>>> }
>>> }
>>> - *eax = *ebx = *ecx = *edx = 0;
>>> + return 0;
>>> +}
>>> +
>>> +void domain_cpuid(
>>> + struct domain *d,
>>> + unsigned int input,
>>> + unsigned int sub_input,
>>> + unsigned int *eax,
>>> + unsigned int *ebx,
>>> + unsigned int *ecx,
>>> + unsigned int *edx)
>>> +{
>>> + if ( !domain_cpuid_exists(d, input, sub_input, eax, ebx, ecx,
>>> edx) )
>>> + *eax = *ebx = *ecx = *edx = 0;
>>> }
>>> void vcpu_kick(struct vcpu *v)
>>> diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
>>> index 15d4b91..08f8038 100644
>>> --- a/xen/arch/x86/sysctl.c
>>> +++ b/xen/arch/x86/sysctl.c
>>> @@ -101,6 +101,23 @@ long arch_do_sysctl(
>>> }
>>> break;
>>> + case XEN_SYSCTL_cpuid_op:
>> This would appear to be a cpuid instruction in the context of a domain,
>> which should be a domctl, not a sysctl. I have a different
>> implementation of sysctl cpuid posted, which takes a pcpu parameter.
>
> No, this is not intended to handle CPUID instruction. This is invoked
> only as part of a sysctl.
>
> As for domctl vs sysctl --- I in fact would have preferred to do this
> via domctl since we already have a data structure to handle this
> (xen_domctl_cpuid). I decided to use sysctl since the intent here is
> to query what the system provides, not what a domain sees.
>
>
>>
>>> + {
>>> + struct xen_sysctl_cpuid *cpuid = &sysctl->u.cpuid;
>>> +
>>> + if ( !cpuid_hypervisor_leaves(cpuid->input[0],
>>> cpuid->input[1],
>>> + &cpuid->eax,&cpuid->ebx,
>>> + &cpuid->ecx, &cpuid->edx) )
>>> + asm ( "cpuid"
>>> + :"=a" (cpuid->eax), "=b" (cpuid->ebx),
>>> + "=c" (cpuid->ecx), "=d" (cpuid->edx)
>>> + : "0" (cpuid->input[0]), "1" (cpuid->input[1]) );
>> This will likely bypass masking/levelling for a domain. As suggested,
>> the hypervisor leaves should be plumbed properly through to be usable
>> from domain_cpuid().
>
> Yes, that was the intent. The thinking here is that we provide to the
> sysctl caller what the actual CPUID is. Now, this (the asm part) is
> not used anywhere so if we limit this sysctl to hypervisor leaves (or
> even to leaf 0x40000001 only as Jan suggested) we won't need this.
>
> (Moreover, for 0x40000001-only approach we may not even need this
> whole sysctl business)
>
> Thanks.
> -boris
All of this smells like it would be substantially more simple under the
proposition in my design to fix heterogeneous feature levelling, where
Xen presents a full and completely CPUID policy for PV and HVM domains
for consumption my the toolstack.
This removes the need for these "bypass the current domains policy so I
can see something about real hardware to build another domain against",
and avoids having libxc create a wrong idea of what it thinks Xen will
provide to the guest, just to have Xen fix up later anyway.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |