Re: [SeaBIOS] [PATCH] KVM vcpu hotplug seabios infrastructure

[Isaku Yamahata]

On Mon, Jun 21, 2010 at 02:52:51PM +0800, Liu, Jinsong wrote:
> Isaku,
>
> Thanks for comments!
> I didn't fine changeset 12cbb43b9dc8a16f712188cb308a0eb0321f3e20, I pull from git://git.kernel.org/pub/scm/virt/kvm/seabios.git, any misunderstanding?

I'm referring to git://git.linuxtogo.org/home/kevin/seabios.git
According to http://www.coreboot.org/SeaBIOS,
it seems that Keven maintains that repo as the seabios upstream.
Kevin when pushing patches, which repository should we refer?
> As for makefile rule for acpi-ssdt-15.hex and acpi-ssdt-anycpu.hex, we in fact combine them into 1 generic rule in our old patch, as attached.
> However, the old makefile require iasl for user compiling seabios.
> Kevin prefer not using iasl, so we update it as new version.

Something like the below didn't work?
(I just wrote it just to show my idea. I didn't any test.)

+src/%.hex: src/acpi-ssdt.dsl src/mk_ssdt.c
+       @echo "Compiling SSDT for general os which support more cpus"
+       $(Q)$(CC) -o mk_$* src/mk_ssdt.c
+       head -n -2 $< > src/$*.dsl.i
+       ./mk_$* >> src/$*.dsl.i
+       $(Q)cpp -P src/$*.dsl.i > src/$*.dsl
+       $(Q)iasl -tc -p $@ src/acpi-ssdt-anycpu.dsl
+       $(Q)sed -i -e 's/AmlCode/$*/g' $@
+       $(Q)rm mk_$* src/$*.dsl.i src/$*.aml src/$*.dsl

thanks,
> Thanks,
> Jinsong
>
> Isaku Yamahata wrote:
> > Hi Jison.
> >
> > Can you check the changeset of
> > 12cbb43b9dc8a16f712188cb308a0eb0321f3e20 and update Makefile changes?
> >
> > The rule for acpi-ssdt-15.hex and acpi-ssdt-anycpu.hex seems
> > same, so it can be combined into a single generic rule.
> > The rules removes the files at the last, but it would be more
> > reliable to use Makefile intermediate and let make to remove
> > them automatically.
> >
> > On Sun, Jun 20, 2010 at 11:40:02PM +0800, Liu, Jinsong wrote:
> > > Kevin O'Connor wrote:
> > > > On Wed, Jun 16, 2010 at 12:19:24PM +0800, Liu, Jinsong wrote:
> > > > > Gleb Natapov wrote:
> > > > > > FWIW looks good to me. This patch removes dynamic creation of
> > > > > > Processor scope and supports only 15/255 cpus configuration, but
> > > > > > given that new code is much more complex that old one I don't see
> > > > > > other possibilities (unless we'll add aml compiler to seabios
> > > > > > runtime).
> > > > I'm okay with the approach (two SSDTs selected by cpu count).  The
> > > > build changes don't look right (15.c is too cryptic).  However, more
> > > > importantly, I want to see where we end up going with the
> > > > qemu/seabios acpi interface discussion before committing this.
> > > >
> > > > -Kevin
> > >
> > > Kevin,
> > >
> > > Thanks for comments!
> > >
> > > In your last comments, you prefer not to use iasl to compile seabios.
> > > Now I update the patch as attached (only need update Makefile):
> > > 1. cancel cryptic name like 15.c from Makefile, and we give detailed
> > > description for cpu number in new version; 
> > > 2. user don't need iasl again to compile seabios, except if they
> > > want to change dsdt/ssdt itself;
> > >
> > > The process to apply patch and build in my desktop is:
> > > $ git apply kvm-vcpu-hotplug-seabios.patch
> > > $ git add src/acpi-ssdt.dsl
> > > $ git add src/mk_ssdt.c
> > > $ git add src/acpi-ssdt-15.hex
> > > $ git add src/acpi-ssdt-anycpu.hex
> > > $ git commit -a
> > > $ make
> > >
> > > We test the updated patch, it works OK with linux2.6.32 hot-add and
> > > hot-remove.
> > >
> > > Thanks,
> > > Jinsong
> > > _______________________________________________
> > > SeaBIOS mailing list
> > > [e-mail]
> > > http://www.seabios.org/mailman/listinfo/seabios

[PATCH] KVM vcpu hotplug seabios infrastructure

[Peter Stuge]

Isaku Yamahata wrote:
> Something like the below didn't work?
> (I just wrote it just to show my idea. I didn't any test.)
>
> +src/%.hex: src/acpi-ssdt.dsl src/mk_ssdt.c
> +       @echo "Compiling SSDT for general os which support more cpus"
> +       $(Q)$(CC) -o mk_$* src/mk_ssdt.c

Maybe the mk_ssdt.c tool could also be built just once if it is
changed to take an integer parameter, instead of deriving the loop
count from the executable name.


//Peter

[PATCH] KVM vcpu hotplug seabios infrastructure

[Kevin O'Connor]

On Mon, Jun 21, 2010 at 04:03:28PM +0900, Isaku Yamahata wrote:
> On Mon, Jun 21, 2010 at 02:52:51PM +0800, Liu, Jinsong wrote:
> > Isaku,
> >
> > Thanks for comments!
> > I didn't fine changeset 12cbb43b9dc8a16f712188cb308a0eb0321f3e20, I pull from git://git.kernel.org/pub/scm/virt/kvm/seabios.git, any misunderstanding?
>
> I'm referring to git://git.linuxtogo.org/home/kevin/seabios.git
> According to http://www.coreboot.org/SeaBIOS,
> it seems that Keven maintains that repo as the seabios upstream.
> Kevin when pushing patches, which repository should we refer?

Patches sent to me should be based off of the linuxtogo repo.

-Kevin

[PATCH] KVM vcpu hotplug seabios infrastructure

[Liu, Jinsong]

Kevin O'Connor wrote:
> > I'm referring to git://git.linuxtogo.org/home/kevin/seabios.git
> > According to http://www.coreboot.org/SeaBIOS,
> > it seems that Keven maintains that repo as the seabios upstream.
> > Kevin when pushing patches, which repository should we refer?
>
> Patches sent to me should be based off of the linuxtogo repo.
>
> -Kevin

Kevin,

Attached is updated patch:
1). rebase my patch referring the linuxtogo repo git://git.linuxtogo.org/home/kevin/seabios.git;
2). According to Isaku and Peter's comments, I update Makefile, use 1 rule to build both acpi_ssdt_15.hex and acpi_ssdt_anycpu.hex, while user don't need iasl to compile seabios;

Thanks Isaku, Kevin, and Peter for your comments!

Regards,
Jinsong

[PATCH] KVM vcpu hotplug seabios infrastructure

[Liu, Jinsong]

Isaku Yamahata wrote:
> I'm referring to git://git.linuxtogo.org/home/kevin/seabios.git
> According to http://www.coreboot.org/SeaBIOS,
> it seems that Keven maintains that repo as the seabios upstream.
> Kevin when pushing patches, which repository should we refer?

I pull from git://git.linuxtogo.org/home/kevin/seabios.git, it has changeset 12cbb43b9dc8a16f712188cb308a0eb0321f3e20, thanks!
> Something like the below didn't work?
> (I just wrote it just to show my idea. I didn't any test.)
>
> +src/%.hex: src/acpi-ssdt.dsl src/mk_ssdt.c
> +       @echo "Compiling SSDT for general os which support more cpus"
> +       $(Q)$(CC) -o mk_$* src/mk_ssdt.c
> +       head -n -2 $< > src/$*.dsl.i
> +       ./mk_$* >> src/$*.dsl.i
> +       $(Q)cpp -P src/$*.dsl.i > src/$*.dsl
> +       $(Q)iasl -tc -p $@ src/acpi-ssdt-anycpu.dsl
> +       $(Q)sed -i -e 's/AmlCode/$*/g' $@
> +       $(Q)rm mk_$* src/$*.dsl.i src/$*.aml src/$*.dsl

If so, current dependency 'src/%.hex: src/%.dsl' will conflict with new dependency 'src/%.hex: src/acpi-ssdt.dsl src/mk_ssdt.c', like when build src/acpi-ssdt.hex?

BTW, in current Makefile, seems the dependency '$(OUT)ccode32flat.o: src/acpi-dsdt.hex' is redundant.
Without it, makefile still build src/acpi-dsdt.hex. I checked the Makefile but didn't find the reason.

Thanks,
Jinsong