Switch to the header we imported from Linux,
this allows us to drop a hack in kvm_i386.h.
More code will be dropped in the next patch.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/sysemu/kvm.h | 1 -
target/i386/cpu.h | 2 --
target/i386/kvm_i386.h | 6 ------
hw/i386/kvm/clock.c | 2 +-
target/i386/cpu.c | 4 +---
target/i386/kvm.c | 4 ++--
6 files changed, 4 insertions(+), 15 deletions(-)
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 23669c4..0b64b8e 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -22,7 +22,6 @@
#ifdef NEED_CPU_H
# ifdef CONFIG_KVM
# include <linux/kvm.h>
-# include <linux/kvm_para.h>
# define CONFIG_KVM_IS_POSSIBLE
# endif
#else
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 8ac13f6..6645046 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -688,8 +688,6 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
#define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */
#define CPUID_7_0_EDX_SPEC_CTRL_SSBD (1U << 31) /* Speculative Store Bypass Disable */
-#define KVM_HINTS_DEDICATED (1U << 0)
-
#define CPUID_8000_0008_EBX_IBPB (1U << 12) /* Indirect Branch Prediction Barrier */
#define CPUID_XSAVE_XSAVEOPT (1U << 0)
diff --git a/target/i386/kvm_i386.h b/target/i386/kvm_i386.h
index 1de9876..e5df24c 100644
--- a/target/i386/kvm_i386.h
+++ b/target/i386/kvm_i386.h
@@ -30,12 +30,6 @@
#define kvm_pic_in_kernel() 0
#define kvm_ioapic_in_kernel() 0
-/* These constants must never be used at runtime if kvm_enabled() is false.
- * They exist so we don't need #ifdefs around KVM-specific code that already
- * checks kvm_enabled() properly.
- */
-#define KVM_CPUID_FEATURES 0
-
#endif /* CONFIG_KVM */
bool kvm_allows_irq0_override(void);
diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
index 7dac319..0bf1c60 100644
--- a/hw/i386/kvm/clock.c
+++ b/hw/i386/kvm/clock.c
@@ -26,7 +26,7 @@
#include "qapi/error.h"
#include <linux/kvm.h>
-#include <linux/kvm_para.h>
+#include "standard-headers/asm-x86/kvm_para.h"
#define TYPE_KVM_CLOCK "kvmclock"
#define KVM_CLOCK(obj) OBJECT_CHECK(KVMClockState, (obj), TYPE_KVM_CLOCK)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index d95310f..9426041 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -40,9 +40,7 @@
#include "qom/qom-qobject.h"
#include "sysemu/arch_init.h"
-#if defined(CONFIG_KVM)
-#include <linux/kvm_para.h>
-#endif
+#include "standard-headers/asm-x86/kvm_para.h"
#include "sysemu/sysemu.h"
#include "hw/qdev-properties.h"
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 0c656a9..6511329 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -18,7 +18,7 @@
#include <sys/utsname.h>
#include <linux/kvm.h>
-#include <linux/kvm_para.h>
+#include "standard-headers/asm-x86/kvm_para.h"
#include "qemu-common.h"
#include "cpu.h"
@@ -387,7 +387,7 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
ret &= ~(1U << KVM_FEATURE_PV_UNHALT);
}
} else if (function == KVM_CPUID_FEATURES && reg == R_EDX) {
- ret |= KVM_HINTS_DEDICATED;
+ ret |= 1U << KVM_HINTS_DEDICATED;
found = 1;
}
--
MST
On 23 May 2018 at 15:43, Michael S. Tsirkin <mst@redhat.com> wrote: > Switch to the header we imported from Linux, > this allows us to drop a hack in kvm_i386.h. > More code will be dropped in the next patch. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- a/target/i386/cpu.h > +++ b/target/i386/cpu.h > @@ -688,8 +688,6 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS]; > #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */ > #define CPUID_7_0_EDX_SPEC_CTRL_SSBD (1U << 31) /* Speculative Store Bypass Disable */ > > -#define KVM_HINTS_DEDICATED (1U << 0) > - > #define CPUID_8000_0008_EBX_IBPB (1U << 12) /* Indirect Branch Prediction Barrier */ > > #define CPUID_XSAVE_XSAVEOPT (1U << 0) Hi -- this seems like it will break compilation when we next update our copy of the Linux kernel headers, because (as of 4.17-rc6, at least), asm-x86/kvm_para.h doesn't define KVM_HINTS_DEDICATED. Here's the diff I get as part of my attempt to run update-linux-headers: --- a/include/standard-headers/asm-x86/kvm_para.h +++ b/include/standard-headers/asm-x86/kvm_para.h @@ -29,7 +29,7 @@ #define KVM_FEATURE_PV_TLB_FLUSH 9 #define KVM_FEATURE_ASYNC_PF_VMEXIT 10 -#define KVM_HINTS_DEDICATED 0 +#define KVM_HINTS_REALTIME 0 /* The last 8 bits are used to indicate how to interpret the flags field * in pvclock structure. If no bits are set, all flags are ignored. I'm not sure what's going on here -- commit 633711e8287 in the kernel just renames the constant, but doesn't that break userspace API ? thanks -- PMM
On 25 May 2018 at 12:06, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 23 May 2018 at 15:43, Michael S. Tsirkin <mst@redhat.com> wrote:
>> Switch to the header we imported from Linux,
>> this allows us to drop a hack in kvm_i386.h.
>> More code will be dropped in the next patch.
>>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
>> --- a/target/i386/cpu.h
>> +++ b/target/i386/cpu.h
>> @@ -688,8 +688,6 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
>> #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */
>> #define CPUID_7_0_EDX_SPEC_CTRL_SSBD (1U << 31) /* Speculative Store Bypass Disable */
>>
>> -#define KVM_HINTS_DEDICATED (1U << 0)
>> -
>> #define CPUID_8000_0008_EBX_IBPB (1U << 12) /* Indirect Branch Prediction Barrier */
>>
>> #define CPUID_XSAVE_XSAVEOPT (1U << 0)
>
> Hi -- this seems like it will break compilation when we next
> update our copy of the Linux kernel headers, because (as of
> 4.17-rc6, at least), asm-x86/kvm_para.h doesn't define
> KVM_HINTS_DEDICATED.
For the moment I'm using this workaround (I wanted to do a header
update for something else I'm working on):
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -48,6 +48,11 @@
#include "exec/memattrs.h"
#include "trace.h"
+/* Work around this kernel header constant changing its name */
+#ifndef KVM_HINTS_REALTIME
+#define KVM_HINTS_REALTIME KVM_HINTS_DEDICATED
+#endif
+
//#define DEBUG_KVM
#ifdef DEBUG_KVM
@@ -387,7 +392,7 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s,
uint32_t function,
ret &= ~(1U << KVM_FEATURE_PV_UNHALT);
}
} else if (function == KVM_CPUID_FEATURES && reg == R_EDX) {
- ret |= 1U << KVM_HINTS_DEDICATED;
+ ret |= 1U << KVM_HINTS_REALTIME;
found = 1;
}
thanks
-- PMM
On Fri, May 25, 2018 at 12:53:44PM +0100, Peter Maydell wrote:
> On 25 May 2018 at 12:06, Peter Maydell <peter.maydell@linaro.org> wrote:
> > On 23 May 2018 at 15:43, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> Switch to the header we imported from Linux,
> >> this allows us to drop a hack in kvm_i386.h.
> >> More code will be dropped in the next patch.
> >>
> >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> >> --- a/target/i386/cpu.h
> >> +++ b/target/i386/cpu.h
> >> @@ -688,8 +688,6 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> >> #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */
> >> #define CPUID_7_0_EDX_SPEC_CTRL_SSBD (1U << 31) /* Speculative Store Bypass Disable */
> >>
> >> -#define KVM_HINTS_DEDICATED (1U << 0)
> >> -
> >> #define CPUID_8000_0008_EBX_IBPB (1U << 12) /* Indirect Branch Prediction Barrier */
> >>
> >> #define CPUID_XSAVE_XSAVEOPT (1U << 0)
> >
> > Hi -- this seems like it will break compilation when we next
> > update our copy of the Linux kernel headers, because (as of
> > 4.17-rc6, at least), asm-x86/kvm_para.h doesn't define
> > KVM_HINTS_DEDICATED.
>
> For the moment I'm using this workaround (I wanted to do a header
> update for something else I'm working on):
>
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -48,6 +48,11 @@
> #include "exec/memattrs.h"
> #include "trace.h"
>
> +/* Work around this kernel header constant changing its name */
> +#ifndef KVM_HINTS_REALTIME
> +#define KVM_HINTS_REALTIME KVM_HINTS_DEDICATED
> +#endif
> +
> //#define DEBUG_KVM
>
> #ifdef DEBUG_KVM
I don't think we need this chunk.
> @@ -387,7 +392,7 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s,
> uint32_t function,
> ret &= ~(1U << KVM_FEATURE_PV_UNHALT);
> }
> } else if (function == KVM_CPUID_FEATURES && reg == R_EDX) {
> - ret |= 1U << KVM_HINTS_DEDICATED;
> + ret |= 1U << KVM_HINTS_REALTIME;
> found = 1;
> }
That's the right change when we update this header.
> thanks
> -- PMM
On 25 May 2018 at 13:18, Michael S. Tsirkin <mst@redhat.com> wrote: > On Fri, May 25, 2018 at 12:53:44PM +0100, Peter Maydell wrote: >> For the moment I'm using this workaround (I wanted to do a header >> update for something else I'm working on): >> >> --- a/target/i386/kvm.c >> +++ b/target/i386/kvm.c >> @@ -48,6 +48,11 @@ >> #include "exec/memattrs.h" >> #include "trace.h" >> >> +/* Work around this kernel header constant changing its name */ >> +#ifndef KVM_HINTS_REALTIME >> +#define KVM_HINTS_REALTIME KVM_HINTS_DEDICATED >> +#endif >> + >> //#define DEBUG_KVM >> >> #ifdef DEBUG_KVM > > I don't think we need this chunk. My view is that header update commits should be exactly and only the result of running update-linux-headers, no manual tweaking. If you don't add this chunk before the update, compilation with the update will fail. You can remove the chunk after the update, of course... thanks -- PMM
On Fri, May 25, 2018 at 01:21:24PM +0100, Peter Maydell wrote: > On 25 May 2018 at 13:18, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Fri, May 25, 2018 at 12:53:44PM +0100, Peter Maydell wrote: > >> For the moment I'm using this workaround (I wanted to do a header > >> update for something else I'm working on): > >> > >> --- a/target/i386/kvm.c > >> +++ b/target/i386/kvm.c > >> @@ -48,6 +48,11 @@ > >> #include "exec/memattrs.h" > >> #include "trace.h" > >> > >> +/* Work around this kernel header constant changing its name */ > >> +#ifndef KVM_HINTS_REALTIME > >> +#define KVM_HINTS_REALTIME KVM_HINTS_DEDICATED > >> +#endif > >> + > >> //#define DEBUG_KVM > >> > >> #ifdef DEBUG_KVM > > > > I don't think we need this chunk. > > My view is that header update commits should be exactly and only > the result of running update-linux-headers, no manual tweaking. > If you don't add this chunk before the update, compilation with the > update will fail. You can remove the chunk after the update, of course... > > thanks > -- PMM I see. I guess you did all the work already, do you still need help or will you just go ahead and post it? Or even commit directly, it's a trivial enough patch. -- MST
On 25 May 2018 at 13:27, Michael S. Tsirkin <mst@redhat.com> wrote: > I see. I guess you did all the work already, do you still need help > or will you just go ahead and post it? Or even commit directly, > it's a trivial enough patch. I'll send a series later this afternoon that does an update to 4.17-rc6; it has a couple of other prerequisites in it, to handle __aligned_u64 in the update script (patch already posted this morning, but I'll put it in the series), and to deal with the kernel headers not yet defining VIRTIO_GPU_CAPSET_VIRGL2. thanks -- PMM
On Fri, May 25, 2018 at 01:30:00PM +0100, Peter Maydell wrote: > On 25 May 2018 at 13:27, Michael S. Tsirkin <mst@redhat.com> wrote: > > I see. I guess you did all the work already, do you still need help > > or will you just go ahead and post it? Or even commit directly, > > it's a trivial enough patch. > > I'll send a series later this afternoon that does an update > to 4.17-rc6; it has a couple of other prerequisites in it, > to handle __aligned_u64 in the update script (patch already > posted this morning, but I'll put it in the series), and to deal > with the kernel headers not yet defining VIRTIO_GPU_CAPSET_VIRGL2. You mean like this: http://patchwork.ozlabs.org/patch/907121/ ? > thanks > -- PMM
On 25 May 2018 at 13:35, Michael S. Tsirkin <mst@redhat.com> wrote: > On Fri, May 25, 2018 at 01:30:00PM +0100, Peter Maydell wrote: >> On 25 May 2018 at 13:27, Michael S. Tsirkin <mst@redhat.com> wrote: >> > I see. I guess you did all the work already, do you still need help >> > or will you just go ahead and post it? Or even commit directly, >> > it's a trivial enough patch. >> >> I'll send a series later this afternoon that does an update >> to 4.17-rc6; it has a couple of other prerequisites in it, >> to handle __aligned_u64 in the update script (patch already >> posted this morning, but I'll put it in the series), and to deal >> with the kernel headers not yet defining VIRTIO_GPU_CAPSET_VIRGL2. > > > You mean like this: > http://patchwork.ozlabs.org/patch/907121/ > > ? Yes; I missed that (found a mail thread pointing out the problem but not the patch with the fix); I'll pick it up for the series. thanks -- PMM
On Fri, May 25, 2018 at 12:06:17PM +0100, Peter Maydell wrote: > On 23 May 2018 at 15:43, Michael S. Tsirkin <mst@redhat.com> wrote: > > Switch to the header we imported from Linux, > > this allows us to drop a hack in kvm_i386.h. > > More code will be dropped in the next patch. > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > --- a/target/i386/cpu.h > > +++ b/target/i386/cpu.h > > @@ -688,8 +688,6 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS]; > > #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */ > > #define CPUID_7_0_EDX_SPEC_CTRL_SSBD (1U << 31) /* Speculative Store Bypass Disable */ > > > > -#define KVM_HINTS_DEDICATED (1U << 0) > > - > > #define CPUID_8000_0008_EBX_IBPB (1U << 12) /* Indirect Branch Prediction Barrier */ > > > > #define CPUID_XSAVE_XSAVEOPT (1U << 0) > > Hi -- this seems like it will break compilation when we next > update our copy of the Linux kernel headers, That just means we'll need to update kvm.c when we do it. > because (as of > 4.17-rc6, at least), asm-x86/kvm_para.h doesn't define > KVM_HINTS_DEDICATED. Here's the diff I get as part of > my attempt to run update-linux-headers: > > --- a/include/standard-headers/asm-x86/kvm_para.h > +++ b/include/standard-headers/asm-x86/kvm_para.h > @@ -29,7 +29,7 @@ > #define KVM_FEATURE_PV_TLB_FLUSH 9 > #define KVM_FEATURE_ASYNC_PF_VMEXIT 10 > > -#define KVM_HINTS_DEDICATED 0 > +#define KVM_HINTS_REALTIME 0 > > /* The last 8 bits are used to indicate how to interpret the flags field > * in pvclock structure. If no bits are set, all flags are ignored. > > I'm not sure what's going on here -- commit 633711e8287 in > the kernel just renames the constant, but doesn't that > break userspace API ? > > thanks > -- PMM
On 25/05/2018 13:06, Peter Maydell wrote: > --- a/include/standard-headers/asm-x86/kvm_para.h > +++ b/include/standard-headers/asm-x86/kvm_para.h > @@ -29,7 +29,7 @@ > #define KVM_FEATURE_PV_TLB_FLUSH 9 > #define KVM_FEATURE_ASYNC_PF_VMEXIT 10 > > -#define KVM_HINTS_DEDICATED 0 > +#define KVM_HINTS_REALTIME 0 > > /* The last 8 bits are used to indicate how to interpret the flags field > * in pvclock structure. If no bits are set, all flags are ignored. > > I'm not sure what's going on here -- commit 633711e8287 in > the kernel just renames the constant, but doesn't that > break userspace API ? No, but only because the constant with the old name was not in any released version. It was added in 4.17-rc1. Thanks, Paolo
© 2016 - 2025 Red Hat, Inc.