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

Re: [PATCH V5 04/22] xen/ioreq: Make x86's IOREQ feature common



On 27.01.2021 21:14, Oleksandr wrote:
> On 27.01.21 18:58, Jan Beulich wrote:
>> On 25.01.2021 20:08, Oleksandr Tyshchenko wrote:
>>> --- a/xen/arch/x86/Kconfig
>>> +++ b/xen/arch/x86/Kconfig
>>> @@ -92,6 +92,7 @@ config PV_LINEAR_PT
>>>   
>>>   config HVM
>>>     def_bool !PV_SHIM_EXCLUSIVE
>>> +   select IOREQ_SERVER
>>>     prompt "HVM support"
>>>     ---help---
>> ... the addition moved below the prompt line (could probably
>> be taken care of while committing, if no other need for a v6
>> arises).
> 
> V6 is planned anyway, so will do, but ...
> 
> 
>>
>> (Personally I think this should be
>>
>> config HVM
>>      bool "HVM support"
>>      default !PV_SHIM_EXCLUSIVE
> 
> ... def_bool is changed to default by intention or this is a typo?

No, it's not a typo. "def_bool" is just a shorthand for "bool"
and "default", which is useful when there's no prompt, but
gets abused (in my view at least) in a number of other cases.
But as said ...

>> anyway, but that's nothing you need to care about.)

... here.

>>> --- /dev/null
>>> +++ b/xen/include/asm-x86/ioreq.h
>>> @@ -0,0 +1,37 @@
>>> +/*
>>> + * ioreq.h: Hardware virtual machine assist interface definitions.
>>> + *
>>> + * This is a wrapper which purpose is to not include arch HVM specific 
>>> header
>>> + * from the common code.
>>> + *
>>> + * Copyright (c) 2016 Citrix Systems Inc.
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms and conditions of the GNU General Public License,
>>> + * version 2, as published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope 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/>.
>>> + */
>>> +
>>> +#ifndef __ASM_X86_IOREQ_H__
>>> +#define __ASM_X86_IOREQ_H__
>>> +
>>> +#include <asm/hvm/ioreq.h>
>>> +
>>> +#endif /* __ASM_X86_IOREQ_H__ */
>> Not necessarily for taking care of right away, I think in the
>> longer run this wants wrapping by #ifdef CONFIG_HVM, such that
>> in !HVM builds the dependency on the "chained" header goes
>> away (reducing the amount of rebuilding in incremental builds).
> 
> I don't mind wrapping it right away.

Well, if that doesn't break the !HVM build, I'd of course
appreciate it. I'd expect fallout, though.

Jan



 


Rackspace

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