qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Ani Sinha <ani@anisinha.ca>,
	marcandre.lureau@redhat.com, Igor Mammedov <imammedo@redhat.com>,
	qemu-devel@nongnu.org, Stefan Berger <stefanb@linux.ibm.com>
Subject: Re: [PATCH v2 3/3] tests: acpi: Add updated TPM related tables
Date: Tue, 9 Nov 2021 11:05:28 -0500	[thread overview]
Message-ID: <20211109110331-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <YYqFxgu74BIh0ZjB@redhat.com>

On Tue, Nov 09, 2021 at 02:29:26PM +0000, Daniel P. Berrangé wrote:
> On Tue, Nov 09, 2021 at 09:14:25AM -0500, Michael S. Tsirkin wrote:
> > On Tue, Nov 09, 2021 at 09:01:52AM -0500, Stefan Berger wrote:
> > > The updated TPM related tables have the following additions:
> > > 
> > >    Device (TPM)
> > >    {
> > >        Name (_HID, "MSFT0101" /* TPM 2.0 Security Device */)  // _HID: Hardware ID
> > > +      Name (_STR, "TPM 2.0 Device")  // _STR: Description String
> > > +      Name (_UID, One)  // _UID: Unique ID
> > >        Name (_STA, 0x0F)  // _STA: Status
> > >        Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
> > > 
> > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > Cc: Igor Mammedov <imammedo@redhat.com>
> > > Cc: Ani Sinha <ani@anisinha.ca>
> > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> > > ---
> > >  tests/data/acpi/q35/DSDT.tis.tpm12          | Bin 0 -> 8900 bytes
> > >  tests/data/acpi/q35/DSDT.tis.tpm2           | Bin 0 -> 8921 bytes
> > >  tests/data/acpi/q35/TCPA.tis.tpm12          | Bin 0 -> 50 bytes
> > >  tests/data/acpi/q35/TPM2.tis.tpm2           | Bin 0 -> 76 bytes
> > >  tests/qtest/bios-tables-test-allowed-diff.h |  11 -----------
> > >  5 files changed, 11 deletions(-)
> > 
> > A disadvantage to doing it like this is that git thinks
> > it's ok to replace any empty file with this, so if acpi
> > changed in any way git will happily resolve it
> > replacing it with this version.
> 
> Do we actually need to be storing these binary files in git
> at all ?
> 
> IIUC, the test will do two things
> 
>  - memcmp the expected binary we store, against the new binary
>    we generated.
>  - if they differ, then disassemble both and report the
>    differences in a user friendly-ish way

Second if iasl is installed.

> What if we only stored the sha256 checksum of the binary

Hmm, point being? Same some space?

> *and*
> the dissasembled output in git, never the full binary.

We used to. The issue is that dissasembled output depends
very much on the disassembler. Disassembling both
with the same tool is one way to address this.

> IIUC, that would give us the same level of diagnostic output
> from the test failures. The dissasembled output would then
> give us meaningful patches for reviewers to look at. The
> author wouldn't have to describe the difference in the
> commit message as Stefan has (helpfully) done here.
> 
> 
> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  parent reply	other threads:[~2021-11-09 16:07 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-09 14:01 [PATCH v2 0/3] tpm: Add missing ACPI device identification objects Stefan Berger
2021-11-09 14:01 ` [PATCH v2 1/3] tests: acpi: prepare for updated TPM related tables Stefan Berger
2021-11-09 14:11   ` Michael S. Tsirkin
2021-11-09 14:30     ` Stefan Berger
2021-11-10 12:15       ` Michael S. Tsirkin
2021-11-09 14:01 ` [PATCH v2 2/3] acpi: tpm: Add missing device identification objects Stefan Berger
2021-11-09 14:20   ` Michael S. Tsirkin
2021-11-09 14:26     ` Stefan Berger
2021-11-10 12:17       ` Michael S. Tsirkin
2021-11-09 14:01 ` [PATCH v2 3/3] tests: acpi: Add updated TPM related tables Stefan Berger
2021-11-09 14:14   ` Michael S. Tsirkin
2021-11-09 14:29     ` Daniel P. Berrangé
2021-11-09 14:56       ` Ani Sinha
2021-11-09 15:05         ` Daniel P. Berrangé
2021-11-09 16:14         ` Michael S. Tsirkin
2021-11-09 16:05       ` Michael S. Tsirkin [this message]
2021-11-09 16:16         ` Daniel P. Berrangé
2021-11-09 16:49           ` Ani Sinha
2021-11-09 16:50           ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211109110331-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=ani@anisinha.ca \
    --cc=berrange@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanb@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).