[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 28.01.21 10:06, Jan Beulich wrote:

Hi Jan

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"
... the addition moved below the prompt line (could probably
be taken care of while committing, if no other need for a v6
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.

ok, well, thank you, but FYI playing with default instead of def_bool I noticed the difference in behavior.

I don't understand why, but HVM option disappears from menuconfig somehow... Or I do something completely wrong...

--- /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.

Hmm, I didn't notice a fallout with that change when CONFIG_HVM is not set

+#ifdef CONFIG_HVM
#include <asm/hvm/ioreq.h>



Oleksandr Tyshchenko



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