On Wed, Sep 30, 2020 at 1:37 PM Ani Sinha <a...@anisinha.ca> wrote:
> On Wed, Sep 30, 2020 at 1:34 PM Michael S. Tsirkin <m...@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 <m...@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 <
> m...@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 <
> m...@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 <
>
> > > m...@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
> <
>
> > >
>
> > > > m...@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 <
>
> > >
>
> > > > m...@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 <
>
> > >
>
> > > > a...@anisinha.ca> wrote:
>
> > >
>
> > > >
>
> > >
>
> > > > > > > > > > > > > >
>
> > >
>
> > > >
>
> > >
>
> > > > > > > > > > > > > > In your pull request the following patch
> is
>
> > > completely
>
> > >
>
> > > > screwed up:
>
> > >
>
> > > >
>
> > >
>
> > > > > > > > > > > > > >
>
> > >
>
> > > >
>
> > >
>
> > > > > > > > > > > > > > commit
> cda2006eded0ed91974e1d9e7f9f288e65812a3e
>
> > >
>
> > > >
>
> > >
>
> > > > > > > > > > > > > > Author: Ani Sinha <a...@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-...@redhat.com/
>
> > >
>
> > > > 20200929071948.281157-46-...@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 <a...@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
>
> > >
>
> > > >
>
> > >
>
> > > > > > > > > >
>
> > >
>
> > > >
>
> > >
>
> > > > > > > >
>
> > >
>
> > > >
>
> > >
>
> > > > > >
>
> > >
>
> > > >
>
> > >
>
> > > >
>
> > >
>
> > > >
>
> > >
>
> > > >
>
> > >
>
> > >
>
> > >
>
> > >
>
> >
>
>
