[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/2] target-i386: move asm-x86/hyperv.h to st
From: |
Cornelia Huck |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/2] target-i386: move asm-x86/hyperv.h to standard-headers |
Date: |
Thu, 10 Sep 2015 10:04:26 +0200 |
On Thu, 10 Sep 2015 10:42:31 +0300
"Michael S. Tsirkin" <address@hidden> wrote:
> On Wed, Sep 09, 2015 at 06:34:01PM +0200, Paolo Bonzini wrote:
> > The Hyper-V definitions are an industry standard and can be used
> > from code that is not KVM-specific.
> >
> > The changes to scripts/update-linux-headers.sh are required because there
> > is both an asm-x86/hyperv.h and a linux/hyperv.h file. linux/hyperv.h
> > introduces dependencies on additional Linux uapi headers, so we only
> > want the former.
> >
> > The solution is to make cp_virtio (now renamed to cp_portable) copy
> > one file only, instead of using the "find" command, and call it multiple
> > times. The new function is really just a reindentation of the old one.
> >
> > Signed-off-by: Paolo Bonzini <address@hidden>
>
> I'd rather see a script update, then result of running it
> in a separate patch.
>
> > ---
> > .../standard-headers}/asm-x86/hyperv.h | 10 +-
> > linux-headers/asm-x86/hyperv.h | 253
> > +-----------------------------
> > scripts/update-linux-headers.sh | 79 +++++-----
> > target-i386/kvm.c | 2 +-
> > 4 files changed, 52 insertions(+), 295 deletions(-)
> > copy {linux-headers => include/standard-headers}/asm-x86/hyperv.h (98%)
> > diff --git a/scripts/update-linux-headers.sh
> > b/scripts/update-linux-headers.sh
> > index 7f7b592..2f25e84 100755
> > --- a/scripts/update-linux-headers.sh
> > +++ b/scripts/update-linux-headers.sh
> > @@ -28,39 +28,32 @@ if [ -z "$output" ]; then
> > output="$PWD"
> > fi
> >
> > -cp_virtio() {
> > - from=$1
> > +cp_portable() {
> > + f=$1
> > to=$2
> > - virtio=$(find "$from" -name '*virtio*h' -o -name "input.h" -o -name
> > "pci_regs.h")
> > - if [ "$virtio" ]; then
> > - rm -rf "$to"
> > - mkdir -p "$to"
> > - for f in $virtio; do
> > - if
> > - grep '#include' "$f" | grep -v -e 'linux/virtio' \
> > - -e 'linux/types' \
> > - -e 'stdint' \
> > - -e 'linux/if_ether' \
> > - -e 'sys/' \
> > - > /dev/null
> > - then
> > - echo "Unexpected #include in input file $f".
> > - exit 2
> > - fi
> > -
> > - header=$(basename "$f");
> > - sed -e 's/__u\([0-9][0-9]*\)/uint\1_t/g' \
> > - -e 's/__s\([0-9][0-9]*\)/int\1_t/g' \
> > - -e 's/__le\([0-9][0-9]*\)/uint\1_t/g' \
> > - -e 's/__be\([0-9][0-9]*\)/uint\1_t/g' \
> > - -e 's/<linux\/\([^>]*\)>/"standard-headers\/linux\/\1"/' \
> > - -e 's/__bitwise__//' \
> > - -e 's/__attribute__((packed))/QEMU_PACKED/' \
> > - -e 's/__inline__/inline/' \
> > - -e '/sys\/ioctl.h/d' \
> > - "$f" > "$to/$header";
> > - done
> > + if
> > + grep '#include' "$f" | grep -v -e 'linux/virtio' \
> > + -e 'linux/types' \
> > + -e 'stdint' \
> > + -e 'linux/if_ether' \
> > + -e 'sys/' \
> > + > /dev/null
> > + then
> > + echo "Unexpected #include in input file $f".
> > + exit 2
> > fi
> > +
> > + header=$(basename "$f");
> > + sed -e 's/__u\([0-9][0-9]*\)/uint\1_t/g' \
> > + -e 's/__s\([0-9][0-9]*\)/int\1_t/g' \
> > + -e 's/__le\([0-9][0-9]*\)/uint\1_t/g' \
> > + -e 's/__be\([0-9][0-9]*\)/uint\1_t/g' \
> > + -e 's/<linux\/\([^>]*\)>/"standard-headers\/linux\/\1"/' \
> > + -e 's/__bitwise__//' \
> > + -e 's/__attribute__((packed))/QEMU_PACKED/' \
> > + -e 's/__inline__/inline/' \
> > + -e '/sys\/ioctl.h/d' \
> > + "$f" > "$to/$header";
> > }
> >
> > # This will pick up non-directories too (eg "Kconfig") but we will
> > @@ -68,6 +61,7 @@ cp_virtio() {
> > ARCHLIST=$(cd "$linux/arch" && echo *)
> >
> > for arch in $ARCHLIST; do
> > +
> > # Discard anything which isn't a KVM-supporting architecture
> > if ! [ -e "$linux/arch/$arch/include/asm/kvm.h" ] &&
> > ! [ -e "$linux/arch/$arch/include/uapi/asm/kvm.h" ] ; then
>
> This empty line looks ugly imho.
>
> > @@ -86,14 +80,19 @@ for arch in $ARCHLIST; do
> > for header in kvm.h kvm_para.h; do
> > cp "$tmpdir/include/asm/$header" "$output/linux-headers/asm-$arch"
> > done
> > - if [ $arch = x86 ]; then
> > - cp "$tmpdir/include/asm/hyperv.h" "$output/linux-headers/asm-x86"
> > - fi
> > if [ $arch = powerpc ]; then
> > cp "$tmpdir/include/asm/epapr_hcalls.h"
> > "$output/linux-headers/asm-powerpc/"
> > fi
> >
> > - cp_virtio "$tmpdir/include/asm"
> > "$output/include/standard-headers/asm-$arch"
> > + rm -rf "$output/include/standard-headers/asm-$arch"
> > + mkdir -p "$output/include/standard-headers/asm-$arch"
> > + if [ $arch = s390 ]; then
> > + cp_portable "$tmpdir/include/asm/kvm_virtio.h"
> > "$output/include/standard-headers/asm-s390/"
> > + cp_portable "$tmpdir/include/asm/virtio-ccw.h"
> > "$output/include/standard-headers/asm-s390/"
>
> I think it's possible that s390 will split its virtio files up in
> the future,
I frankly don't see how we'd want to split those up any further.
> or that more architectures will add their own.
Probably unlikely for virtio files: s390 is the only one with two
unique transports.
For other portable headers: possible; but I'd think they're more likely
to appear in architecture-independent code.
> See below for a suggestion.
>
>
> > + fi
> > + if [ $arch = x86 ]; then
> > + cp_portable "$tmpdir/include/asm/hyperv.h"
> > "$output/include/standard-headers/asm-x86/"
> > + fi
> > done
> >
> > rm -rf "$output/linux-headers/linux"
> > @@ -113,6 +112,9 @@ else
> > cp "$linux/COPYING" "$output/linux-headers"
> > fi
> >
> > +cat <<EOF >$output/linux-headers/asm-x86/hyperv.h
> > +#include "standard-headers/asm-x86/hyperv.h"
> > +EOF
>
> I don't think this is needed. We only did this for
> virtio_config to avoid the code churn. Hyperv has
> a single user, so no issue.
>
> > cat <<EOF >$output/linux-headers/linux/virtio_config.h
> > #include "standard-headers/linux/virtio_config.h"
> > EOF
> > @@ -120,7 +122,12 @@ cat <<EOF >$output/linux-headers/linux/virtio_ring.h
> > #include "standard-headers/linux/virtio_ring.h"
> > EOF
> >
> > -cp_virtio "$tmpdir/include/linux/" "$output/include/standard-headers/linux"
> > +rm -rf "$output/include/standard-headers/linux"
> > +mkdir -p "$output/include/standard-headers/linux"
> > +for i in "$tmpdir"/include/linux/*virtio*.h
> > "$tmpdir/include/linux/input.h" \
> > + "$tmpdir/include/linux/pci_regs.h"; do
> > + cp_portable "$i" "$output/include/standard-headers/linux"
> > +done
>
> How about we move the above loop into cp_virtio?
> Then we can reuse it for asm like we did.
> hyperv can use cp_portable if it wants to.
I prefer specifying the files to copy outside of the copying function:
it's much more obvious what's going on. Otherwise, you get "if there's
a file that happens to match this pattern, copy it" - and it's unclear
to the casual reader which of those actually exist, as linux/ and asm/
have different sets of files.
>
> >
> > cat <<EOF >$output/include/standard-headers/linux/types.h
> > #include <stdint.h>
- [Qemu-devel] [PATCH v2 0/2] update-linux-headers changes, Paolo Bonzini, 2015/09/09
- [Qemu-devel] [PATCH v2 2/2] target-i386: move asm-x86/hyperv.h to standard-headers, Paolo Bonzini, 2015/09/09
- Re: [Qemu-devel] [PATCH v2 2/2] target-i386: move asm-x86/hyperv.h to standard-headers, Cornelia Huck, 2015/09/10
- Re: [Qemu-devel] [PATCH v2 2/2] target-i386: move asm-x86/hyperv.h to standard-headers, Michael S. Tsirkin, 2015/09/10
- Re: [Qemu-devel] [PATCH v2 2/2] target-i386: move asm-x86/hyperv.h to standard-headers,
Cornelia Huck <=
- Re: [Qemu-devel] [PATCH v2 2/2] target-i386: move asm-x86/hyperv.h to standard-headers, Paolo Bonzini, 2015/09/10
- Re: [Qemu-devel] [PATCH v2 2/2] target-i386: move asm-x86/hyperv.h to standard-headers, Michael S. Tsirkin, 2015/09/10
- Re: [Qemu-devel] [PATCH v2 2/2] target-i386: move asm-x86/hyperv.h to standard-headers, Paolo Bonzini, 2015/09/10
- Re: [Qemu-devel] [PATCH v2 2/2] target-i386: move asm-x86/hyperv.h to standard-headers, Michael S. Tsirkin, 2015/09/10
- Re: [Qemu-devel] [PATCH v2 2/2] target-i386: move asm-x86/hyperv.h to standard-headers, Paolo Bonzini, 2015/09/10
[Qemu-devel] [PATCH v2 1/2] update Linux headers to 4.2, Paolo Bonzini, 2015/09/09
Re: [Qemu-devel] [PATCH v2 0/2] update-linux-headers changes, Denis V. Lunev, 2015/09/10