[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH for-next v3 9/9] x86: introduce CONFIG_HYPERV and detection code
On 21.11.2019 17:27, Wei Liu wrote: > On Fri, Nov 15, 2019 at 03:07:29PM +0100, Jan Beulich wrote: >> On 21.10.2019 17:57, Wei Liu wrote: >>> --- a/xen/arch/x86/Kconfig >>> +++ b/xen/arch/x86/Kconfig >>> @@ -164,6 +164,15 @@ endchoice >>> config GUEST >>> bool >>> >>> +config HYPERV_GUEST >>> + def_bool n >>> + select GUEST >>> + prompt "Hyper-V Guest" >> >> Please can you avoid following the bad example XEN_GUEST gives (and >> perhaps even take the opportunity here or in the earlier patch >> adding GUEST to change that one as well)? What you want is >> >> config HYPERV_GUEST >> bool "Hyper-V Guest" >> select GUEST > > Ack. > >> >>> --- /dev/null >>> +++ b/xen/arch/x86/guest/hyperv/hyperv.c >>> @@ -0,0 +1,54 @@ >>> +/****************************************************************************** >>> + * arch/x86/guest/hyperv/hyperv.c >>> + * >>> + * Support for detecting and running under Hyper-V. >>> + * >>> + * This program is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License as published by >>> + * the Free Software Foundation; either version 2 of the License, or >>> + * (at your option) any later version. >>> + * >>> + * This program is distributed in the hope that it will be useful, >>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>> + * GNU General Public License for more details. >>> + * >>> + * You should have received a copy of the GNU General Public License >>> + * along with this program; If not, see <http://www.gnu.org/licenses/>. >>> + * >>> + * Copyright (c) 2019 Microsoft. >>> + */ >>> +#include <xen/init.h> >>> + >>> +#include <asm/guest.h> >>> + >>> +bool __init hyperv_probe(void) >>> +{ >>> + uint32_t eax, ebx, ecx, edx; >>> + >>> + cpuid(0x40000000, &eax, &ebx, &ecx, &edx); >>> + if ( !((ebx == 0x7263694d) && /* "Micr" */ >>> + (ecx == 0x666f736f) && /* "osof" */ >>> + (edx == 0x76482074)) ) /* "t Hv" */ >>> + return false; >>> + >>> + cpuid(0x40000001, &eax, &ebx, &ecx, &edx); >>> + if ( eax != 0x31237648 ) /* Hv#1 */ >>> + return false; >>> + >>> + return true; >>> +} >>> + >>> +struct hypervisor_ops hyperv_ops = { >> >> const again. >> >>> --- a/xen/arch/x86/guest/hypervisor.c >>> +++ b/xen/arch/x86/guest/hypervisor.c >>> @@ -43,6 +43,14 @@ bool hypervisor_probe(void) >>> } >>> #endif >>> >>> +#ifdef CONFIG_HYPERV_GUEST >>> + if ( hyperv_probe() ) >>> + { >>> + hops = &hyperv_ops; >>> + return true; >>> + } >>> +#endif >> >> This recurring #ifdef CONFIG_*_GUEST is going to start looking ugly >> the latest when one or two more get added. Perhaps better providing >> *_probe() stubs returning false, and (like we do elsewhere) rely on >> DCE to get rid of the *_ops reference? (And really you already have >> such a stub - all you need to do is put the hyperv_ops declaration >> outside the #ifdef (but read on). >> >> Also how about having *_probe() return the address of *_ops, such >> that the latter could all become static? > > Previously you made a suggestion to make probe return the name of the > hypervisor. Here you ask for address of ops. I actually prefer the > method suggested here, but this means I will need to keep > hypervisor_name around. Is there actually any user of the name field other than the caller of probe? If not, surely that caller could access the name field without a hypervisor_name() wrapper. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |