On Wed, Sep 30, 2020 at 1:34 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Sep 30, 2020 at 01:17:53PM +0530, Ani Sinha wrote:
> >
> >
> > On Wed, Sep 30, 2020 at 1:14 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Wed, Sep 30, 2020 at 01:09:09PM +0530, Ani Sinha wrote:
> >
> > >
> >
> > >
> >
> > > On Wed, Sep 30, 2020 at 1:06 PM Michael S. Tsirkin <mst@redhat.com>
> > wrote:
> >
> > >
> >
> > > On Tue, Sep 29, 2020 at 06:03:00PM +0530, Ani Sinha wrote:
> >
> > >
> >
> > > > On Tue, Sep 29, 2020 at 5:05 PM Michael S. Tsirkin <mst@redhat.com>
> >
> > > wrote:
> >
> > >
> >
> > > > >
> >
> > >
> >
> > > > > On Tue, Sep 29, 2020 at 04:58:03PM +0530, Ani Sinha wrote:
> >
> > >
> >
> > > > > > On Tue, Sep 29, 2020 at 4:45 PM Michael S. Tsirkin <
> > mst@redhat.com>
> >
> > > wrote:
> >
> > >
> >
> > > > > > >
> >
> > >
> >
> > > > > > > On Tue, Sep 29, 2020 at 04:35:50PM +0530, Ani Sinha wrote:
> >
> > >
> >
> > > > > > > > On Tue, Sep 29, 2020 at 4:25 PM Michael S. Tsirkin <
> >
> > > mst@redhat.com> wrote:
> >
> > >
> >
> > > > > > > > >
> >
> > >
> >
> > > > > > > > > On Tue, Sep 29, 2020 at 04:11:45PM +0530, Ani Sinha
> > wrote:
> >
> > >
> >
> > > > > > > > > > On Tue, Sep 29, 2020 at 4:07 PM Michael S. Tsirkin <
> >
> > > mst@redhat.com> wrote:
> >
> > >
> >
> > > > > > > > > > >
> >
> > >
> >
> > > > > > > > > > > On Tue, Sep 29, 2020 at 04:02:07PM +0530, Ani Sinha
> > wrote:
> >
> > >
> >
> > > > > > > > > > > > On Tue, Sep 29, 2020 at 4:00 PM Ani Sinha <
> >
> > > ani@anisinha.ca> wrote:
> >
> > >
> >
> > > > > > > > > > > > >
> >
> > >
> >
> > > > > > > > > > > > > In your pull request the following patch is
> > completely
> >
> > > screwed up:
> >
> > >
> >
> > > > > > > > > > > > >
> >
> > >
> >
> > > > > > > > > > > > > commit cda2006eded0ed91974e1d9e7f9f288e65812a3e
> >
> > >
> >
> > > > > > > > > > > > > Author: Ani Sinha <ani@anisinha.ca>
> >
> > >
> >
> > > > > > > > > > > > > Date: Tue Sep 29 03:22:52 2020 -0400
> >
> > >
> >
> > > > > > > > > > > > >
> >
> > >
> >
> > > > > > > > > > > > > tests/acpi: update golden master DSDT binary
> > table
> >
> > > blobs for q35
> >
> > >
> >
> > > > > > > > > > > > >
> >
> > >
> >
> > > > > > > > > > > > >
> >
> > >
> >
> > > > > > > > > > > > > This is not my patch. It has all sorts of changes
> > which
> >
> > > does not
> >
> > >
> >
> > > > > > > > > > > > > belong there. Can you please check?
> >
> > >
> >
> > > > > > > > > > > >
> >
> > >
> >
> > > > > > > > > > > > See https://patchew.org/QEMU/
> >
> > > 20200929071948.281157-1-mst@redhat.com/" rel="noreferrer" target="_blank">20200929071948.281157-1-mst@redhat.com/
> >
> > > 20200929071948.281157-46-mst@redhat.com/" rel="noreferrer" target="_blank">20200929071948.281157-46-mst@redhat.com/
> >
> > >
> >
> > > > > > > > > > >
> >
> > >
> >
> > > > > > > > > > >
> >
> > >
> >
> > > > > > > > > > > I had to regenerate the binary, yes. That's par for
> > the
> >
> > > course.
> >
> > >
> >
> > > > > > > > > > > But it looks like I added disasssembled files. Will
> > fix up
> >
> > > and drop,
> >
> > >
> >
> > > > > > > > > > > thanks for noticing this.
> >
> > >
> >
> > > > > > > > >
> >
> > >
> >
> > > > > > > > > OK I pushed out a fixed variant. Pls take a look.
> >
> > >
> >
> > > > > > > >
> >
> > >
> >
> > > > > > > > OK I am not used to this workflow. How am I supposed to get
> > it?
> >
> > > Which tag?
> >
> > >
> >
> > > > > > >
> >
> > >
> >
> > > > > > > New for_upstream tag - I just sent in a pull request.
> >
> > >
> >
> > > > > >
> >
> > >
> >
> > > > > > Can you please point me to your tree?
> >
> > >
> >
> > > > >
> >
> > >
> >
> > > > >
> >
> > >
> >
> > > > > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/
> > for_upstream
> >
> > >
> >
> > > >
> >
> > >
> >
> > > > I have sent the updated patches based on your pull request tag. I
> > just
> >
> > >
> >
> > > > had to regenrated the blob for tests/data/acpi/pc/DSDT.hpbrroot.
> >
> > >
> >
> > > >
> >
> > >
> >
> > > > make && make check-qtest-x86_64 V=1 passes.
> >
> > >
> >
> > > >
> >
> > >
> >
> > > > The diff looks good.
> >
> > >
> >
> > > >
> >
> > >
> >
> > > > Can you please send a pull request with these two patches ASAP?
> >
> > >
> >
> > >
> >
> > >
> >
> > >
> >
> > >
> >
> > > Thanks, I will queue them and merge in the next pull request.
> >
> > >
> >
> > >
> >
> > > I'm willing to get down on my knees begging you to just do one another
> > pull
> >
> > > request for these two patches. Were so close with my entire work merged.
> >
> > >
> >
> > > Please let's not wait another week or so.
> >
> >
> >
> >
> >
> > OK it's not too much work but ... could you please add justification
> >
> > about why adding this one unit test is needed so urgently?
> >
> > That motivation would be quite helpful for the pull request.
> >
> >
> > A patch without unit test doesn't complete the patch work. A unit test makes
> > sure that the change would not get broken by other changes that come in later.
> > Typically all code changes are accompanied by unit test in the same patch.
> > Hence since the main work has already been merged, the unit test should merge
> > ASAP so that no breakage can happen in between.
> >
> > Plus this completes an entire series of work which I've been working for a
> > while. I really would love to see it all merged cleanly and fully completed.
> >
>
> Absolutely, thanks for the great work!
> I am not sure that's a good justification to rushing a pull request
> though ... are you waiting to get paid and it hinges on the test, or are
> under a deadline, or something like this? It's okay to say so if so.
Yes I am under a deadline too. Unfortunately can't disclose more details.
Thanks Michael. Very much appreciate your help here.
>
> >
> >
> >
> >
> >
> >
> > >
> >
> > >
> >
> > >
> >
> > >
> >
> > >
> >
> > > > >
> >
> > >
> >
> > > > >
> >
> > >
> >
> > > > >
> >
> > >
> >
> > > > > > >
> >
> > >
> >
> > > > > > > >
> >
> > >
> >
> > > > > > > > >
> >
> > >
> >
> > > > > > > > > > I think DSDT.hbridge is wrong. The checksum looks
> > weird:
> >
> > >
> >
> > > > > > > > > >
> >
> > >
> >
> > > > > > > > > >
> >
> > >
> >
> > > > > > > > > > + * Length 0x00000B89 (2953)
> >
> > >
> >
> > > > > > > > > > * Revision 0x01 **** 32-bit table (V1),
> > no
> >
> > > 64-bit math support
> >
> > >
> >
> > > > > > > > > > - * Checksum 0x05
> >
> > >
> >
> > > > > > > > >
> >
> > >
> >
> > > > > > > > > What is weird about it?
> >
> > >
> >
> > > > > > > > >
> >
> > >
> >
> > > > > > > > > >
> >
> > >
> >
> > > > > > > > > > This file should be introduced just by one patch. my
> > patch.
> >
> > >
> >
> > > > > > > > >
> >
> > >
> >
> > > > > > > > > I just re-run rebuild-expected-aml, no changes.
> >
> > >
> >
> > > > > > > > >
> >
> > >
> >
> > > > > > > > > I have this:
> >
> > >
> >
> > > > > > > > > commit 5e3a486211f02d9ecb18939ca21087515ec81883
> >
> > >
> >
> > > > > > > > > Author: Ani Sinha <ani@anisinha.ca>
> >
> > >
> >
> > > > > > > > > Date: Fri Sep 18 14:11:05 2020 +0530
> >
> > >
> >
> > > > > > > > >
> >
> > >
> >
> > > > > > > > > tests/acpi: unit test for
> >
> > > 'acpi-pci-hotplug-with-bridge-support' bridge flag
> >
> > >
> >
> > > > > > > > >
> >
> > >
> >
> > > > > > > > > This change adds a new unit test for the global flag
> >
> > >
> >
> > > > > > > > > 'acpi-pci-hotplug-with-bridge-support' which is
> > available
> >
> > > for cold plugged pci
> >
> > >
> >
> > > > > > > > > bridges in i440fx. The flag can be used to turn off
> > ACPI
> >
> > > based hotplug support
> >
> > >
> >
> > > > > > > > > on all pci bridges.
> >
> > >
> >
> > > > > > > > >
> >
> > >
> >
> > > > > > > > >
> >
> > >
> >
> > > > > > > > > Here is the full DSDT header, attached:
> >
> > >
> >
> > > > > > > > >
> >
> > >
> >
> > > > > > > > > /*
> >
> > >
> >
> > > > > > > > > * Intel ACPI Component Architecture
> >
> > >
> >
> > > > > > > > > * AML/ASL+ Disassembler version 20190509 (64-bit
> > version)
> >
> > >
> >
> > > > > > > > > * Copyright (c) 2000 - 2019 Intel Corporation
> >
> > >
> >
> > > > > > > > > *
> >
> > >
> >
> > > > > > > > > * Disassembling to symbolic ASL+ operators
> >
> > >
> >
> > > > > > > > > *
> >
> > >
> >
> > > > > > > > > * Disassembly of tests/data/acpi/pc/DSDT.hpbridge, Tue
> > Sep 29
> >
> > > 06:51:03 2020
> >
> > >
> >
> > > > > > > > > *
> >
> > >
> >
> > > > > > > > > * Original Table Header:
> >
> > >
> >
> > > > > > > > > * Signature "DSDT"
> >
> > >
> >
> > > > > > > > > * Length 0x0000139D (5021)
> >
> > >
> >
> > > > > > > > > * Revision 0x01 **** 32-bit table (V1), no
> > 64-bit
> >
> > > math support
> >
> > >
> >
> > > > > > > > > * Checksum 0x05
> >
> > >
> >
> > > > > > > > > * OEM ID "BOCHS "
> >
> > >
> >
> > > > > > > > > * OEM Table ID "BXPCDSDT"
> >
> > >
> >
> > > > > > > > > * OEM Revision 0x00000001 (1)
> >
> > >
> >
> > > > > > > > > * Compiler ID "BXPC"
> >
> > >
> >
> > > > > > > > > * Compiler Version 0x00000001 (1)
> >
> > >
> >
> > > > > > > > > */
> >
> > >
> >
> > > > > > > > > DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPCDSDT",
> >
> > > 0x00000001)
> >
> > >
> >
> > > > > > > > >
> >
> > >
> >
> > > > > > > > > --
> >
> > >
> >
> > > > > > > > > MST
> >
> > >
> >
> > > > > > > > >
> >
> > >
> >
> > > > > > >
> >
> > >
> >
> > > > >
> >
> > >
> >
> > >
> >
> > >
> >
> > >
> >
> >
> >
> >
>