| Message ID | CAFEAcA9mAMCznsf1hao2PFziXnATrCxa8Of33NckUGXxFU6yEQ@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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
--- 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; }