On Thu, Sep 10, 2015 at 11:19:48AM +0200, Paolo Bonzini wrote:
>
>
> On 10/09/2015 09:42, Michael S. Tsirkin 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 <pbonz...@redhat.com>
> >
> > I'd rather see a script update, then result of running it
> > in a separate patch.
>
> Okay, I'll split the patch in two.
>
> >> @@ -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.
>
> Right.
>
> >> +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.
>
> It is needed because kvm headers include <asm/hyperv.h> and you don't
> want to pick the system version.
I see. Makes sense. Then maybe don't bother with kvm.c either?
Keeps it consistent ...
> >> 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?
>
> I disagree, because input.h and pci_regs.h make no sense for s390.
>
> Paolo
And Cornelia thinks s390 won't need anything new ever, either.
OK then.
> > Then we can reuse it for asm like we did.
> > hyperv can use cp_portable if it wants to.
> >
> >>
> >> cat <<EOF >$output/include/standard-headers/linux/types.h
> >> #include <stdint.h>
> >> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> >> index 066d03d..d8a11be 100644
> >> --- a/target-i386/kvm.c
> >> +++ b/target-i386/kvm.c
> >> @@ -33,7 +33,7 @@
> >> #include "hw/i386/apic_internal.h"
> >> #include "hw/i386/apic-msidef.h"
> >> #include "exec/ioport.h"
> >> -#include <asm/hyperv.h>
> >> +#include "standard-headers/asm-x86/hyperv.h"
> >> #include "hw/pci/pci.h"
> >> #include "migration/migration.h"
> >> #include "exec/memattrs.h"
> >> --
> >> 2.4.3
