| Message ID | 1459758556-4557-2-git-send-email-james.hogan@imgtec.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Apr 04, 2016 at 09:29:15AM +0100, James Hogan wrote: > The cacheflush system call (found on MIPS and ARM) has been included in > the libseccomp header since 2.2.0, so include include it back to that > version. Previously it was only enabled since 2.2.3 since that is when > it was enabled properly for ARM. > > This will allow seccomp support to be enabled for MIPS back to > libseccomp 2.2.0. > > Signed-off-by: James Hogan <james.hogan@imgtec.com> > Cc: Eduardo Otubo <eduardo.otubo@profitbricks.com> > Cc: Aurelien Jarno <aurelien@aurel32.net> > --- > qemu-seccomp.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-By: Andrew Jones <drjones@redhat.com> > > diff --git a/qemu-seccomp.c b/qemu-seccomp.c > index 2866e3c2a660..9425dac17155 100644 > --- a/qemu-seccomp.c > +++ b/qemu-seccomp.c > @@ -18,9 +18,7 @@ > > #if SCMP_VER_MAJOR >= 3 > #define HAVE_CACHEFLUSH > -#elif SCMP_VER_MAJOR == 2 && SCMP_VER_MINOR >= 3 > - #define HAVE_CACHEFLUSH > -#elif SCMP_VER_MAJOR == 2 && SCMP_VER_MINOR == 2 && SCMP_VER_MICRO >= 3 > +#elif SCMP_VER_MAJOR == 2 && SCMP_VER_MINOR >= 2 > #define HAVE_CACHEFLUSH > #endif > > -- > 2.4.10 > >
On 8 April 2016 at 12:45, Andrew Jones <drjones@redhat.com> wrote: > On Mon, Apr 04, 2016 at 09:29:15AM +0100, James Hogan wrote: >> The cacheflush system call (found on MIPS and ARM) has been included in >> the libseccomp header since 2.2.0, so include include it back to that >> version. Previously it was only enabled since 2.2.3 since that is when >> it was enabled properly for ARM. >> >> This will allow seccomp support to be enabled for MIPS back to >> libseccomp 2.2.0. >> >> Signed-off-by: James Hogan <james.hogan@imgtec.com> >> Cc: Eduardo Otubo <eduardo.otubo@profitbricks.com> >> Cc: Aurelien Jarno <aurelien@aurel32.net> >> --- >> qemu-seccomp.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) > > Reviewed-By: Andrew Jones <drjones@redhat.com> Andrew: when we originally put this ifdeffery in you said "libseccomp didn't know about cacheflush until 2.2.1, and we require 2.2.3 for other reasons" (https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg07258.html) Has something changed, were you just wrong back then, or have you forgotten something this time around? (Do you remember what the "other reasons" were?) thanks -- PMM
On Fri, Apr 08, 2016 at 12:54:54PM +0100, Peter Maydell wrote: > On 8 April 2016 at 12:45, Andrew Jones <drjones@redhat.com> wrote: > > On Mon, Apr 04, 2016 at 09:29:15AM +0100, James Hogan wrote: > >> The cacheflush system call (found on MIPS and ARM) has been included in > >> the libseccomp header since 2.2.0, so include include it back to that > >> version. Previously it was only enabled since 2.2.3 since that is when > >> it was enabled properly for ARM. > >> > >> This will allow seccomp support to be enabled for MIPS back to > >> libseccomp 2.2.0. > >> > >> Signed-off-by: James Hogan <james.hogan@imgtec.com> > >> Cc: Eduardo Otubo <eduardo.otubo@profitbricks.com> > >> Cc: Aurelien Jarno <aurelien@aurel32.net> > >> --- > >> qemu-seccomp.c | 4 +--- > >> 1 file changed, 1 insertion(+), 3 deletions(-) > > > > Reviewed-By: Andrew Jones <drjones@redhat.com> > > Andrew: when we originally put this ifdeffery in you said > "libseccomp didn't know about cacheflush until 2.2.1, and we > require 2.2.3 for other reasons" > (https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg07258.html) In that message I was referring to arm/aarch64 (the only arch at the time that cared about cacheflush). 2.2.0 was the first version arm got any cacheflush support (same version as x86 an mips), but it was completely wrong until 2.2.1. Additional problems were then fixed for 2.2.3. x86 doesn't use cacheflush and mips didn't have any problems (I guess), so they've been fine since 2.2.0. As long as the configure script still requires a libseccomp_minver of 2.2.3 for arm/aarch64, then it doesn't matter if the HAVE_CACHEFLUSH minimum version in qemu-seccomp.c is relaxed for the other architectures that work. > > Has something changed, were you just wrong back then, or have > you forgotten something this time around? (Do you remember what > the "other reasons" were?) One reason was that the syscalls for ARM were assuming only a __NR_ prefix instead of __ARM_NR_. And I think there was yet another issue too, but I can't recall that one now. Thanks, drew
On 8 April 2016 at 13:39, Andrew Jones <drjones@redhat.com> wrote: > In that message I was referring to arm/aarch64 (the only arch at the > time that cared about cacheflush). 2.2.0 was the first version arm got > any cacheflush support (same version as x86 an mips), but it was > completely wrong until 2.2.1. Additional problems were then fixed for > 2.2.3. x86 doesn't use cacheflush and mips didn't have any problems > (I guess), so they've been fine since 2.2.0. > > As long as the configure script still requires a libseccomp_minver of > 2.2.3 for arm/aarch64, then it doesn't matter if the HAVE_CACHEFLUSH > minimum version in qemu-seccomp.c is relaxed for the other > architectures that work. OK. Could we have a comment, maybe something like: /* For some architectures (notably ARM) cacheflush is not * supported until libseccomp 2.2.3, but configure enforces that * we are using a more recent version on those hosts, so it is OK * for this check to be less strict. */ ? (or feel free to reword if you have a better idea). thanks -- PMM
On Fri, Apr 08, 2016 at 01:49:44PM +0100, Peter Maydell wrote: > On 8 April 2016 at 13:39, Andrew Jones <drjones@redhat.com> wrote: > > In that message I was referring to arm/aarch64 (the only arch at the > > time that cared about cacheflush). 2.2.0 was the first version arm got > > any cacheflush support (same version as x86 an mips), but it was > > completely wrong until 2.2.1. Additional problems were then fixed for > > 2.2.3. x86 doesn't use cacheflush and mips didn't have any problems > > (I guess), so they've been fine since 2.2.0. > > > > As long as the configure script still requires a libseccomp_minver of > > 2.2.3 for arm/aarch64, then it doesn't matter if the HAVE_CACHEFLUSH > > minimum version in qemu-seccomp.c is relaxed for the other > > architectures that work. > > OK. Could we have a comment, maybe something like: > /* For some architectures (notably ARM) cacheflush is not > * supported until libseccomp 2.2.3, but configure enforces that > * we are using a more recent version on those hosts, so it is OK > * for this check to be less strict. > */ > > ? (or feel free to reword if you have a better idea). Sure, I'll do a v2. Cheers James
diff --git a/qemu-seccomp.c b/qemu-seccomp.c index 2866e3c2a660..9425dac17155 100644 --- a/qemu-seccomp.c +++ b/qemu-seccomp.c @@ -18,9 +18,7 @@ #if SCMP_VER_MAJOR >= 3 #define HAVE_CACHEFLUSH -#elif SCMP_VER_MAJOR == 2 && SCMP_VER_MINOR >= 3 - #define HAVE_CACHEFLUSH -#elif SCMP_VER_MAJOR == 2 && SCMP_VER_MINOR == 2 && SCMP_VER_MICRO >= 3 +#elif SCMP_VER_MAJOR == 2 && SCMP_VER_MINOR >= 2 #define HAVE_CACHEFLUSH #endif
The cacheflush system call (found on MIPS and ARM) has been included in the libseccomp header since 2.2.0, so include include it back to that version. Previously it was only enabled since 2.2.3 since that is when it was enabled properly for ARM. This will allow seccomp support to be enabled for MIPS back to libseccomp 2.2.0. Signed-off-by: James Hogan <james.hogan@imgtec.com> Cc: Eduardo Otubo <eduardo.otubo@profitbricks.com> Cc: Aurelien Jarno <aurelien@aurel32.net> --- qemu-seccomp.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)