On Tue, 21 May 2019 10:34:03 -0400
"Michael S. Tsirkin" <m...@redhat.com> wrote:
> On Tue, May 21, 2019 at 04:29:58PM +0200, Igor Mammedov wrote:
> > On Mon, 20 May 2019 23:37:22 -0400
> > "Michael S. Tsirkin" <m...@redhat.com> wrote:
> >
> > > On Tue, May 21, 2019 at 11:32:49AM +0800, Wei Yang wrote:
> > > > build_append_foo() API doesn't need explicit endianness conversions
> > > > which eliminates a source of errors and it makes build_mcfg() look like
> > > > declarative definition of MCFG table in ACPI spec, which makes it easy
> > > > to review.
> > > >
> > > > Signed-off-by: Wei Yang <richardw.y...@linux.intel.com>
> > > > Suggested-by: Igor Mammedov <imamm...@redhat.com>
> > > > Reviewed-by: Igor Mammedov <imamm...@redhat.com>
> > > >
> > > > ---
> > > > v5:
> > > > * miss the reserved[8] of MCFG in last version, add it back
> > > > * drop SOBs and make sure bios-tables-test all OK
> > > > ---
> > >
> > >
> > > Please do not add two --- separators. It breaks git am.
> > that worked just fine for the last xx years and still works for me.
>
> Because you don't use git am?
> It's true I often am forced to edit your patches to apply them.
> Pls do not do this it makes no sense.
I do not use it as much as you, but I do use git am to test patches from
list and it works with double separator as expected (the second separator
is ignored).
> > > One --- should come after the commit log. Anything after that and
> > > until diff is ignored anyway.
> > That's fine if we wish to commit change log into history, but
> > Typically we don't want change log to be committed (as it's useless from
> > history pov),
> > hence we put it after separator to get it dropped on applying.
>
> Then you should do
the problem is that the second separator is added by git, so one
would have to edit it out after format-patch (which is error prone in
case of multi-patch series).
I don't know much about git though, so could you suggest how to handle
situation to avoid the second --- (i.e. how do you do it)?
PS:
There is 'git notes' to add/track per patch annotations, but it a bit
tedious to use notes compared to in-lining change log in commit message.
> build_append_foo() API doesn't need explicit endianness conversions
> which eliminates a source of errors and it makes build_mcfg() look like
> declarative definition of MCFG table in ACPI spec, which makes it easy
> to review.
>
> Signed-off-by: Wei Yang <richardw.y...@linux.intel.com>
> Suggested-by: Igor Mammedov <imamm...@redhat.com>
> Reviewed-by: Igor Mammedov <imamm...@redhat.com>
>
> ---
> v5:
> * miss the reserved[8] of MCFG in last version, add it back
> * drop SOBs and make sure bios-tables-test all OK
>
> hw/acpi/pci.c | 35 +++++++++++++++++++++++------------
> include/hw/acpi/acpi-defs.h | 18 ------------------
> 2 files changed, 23 insertions(+), 30 deletions(-)
>
> diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
>
>
> Anything between --- and diff is ignored. Second --- is poointless and
> harmful.
>
> > >
> > > > hw/acpi/pci.c | 35 +++++++++++++++++++++++------------
> > > > include/hw/acpi/acpi-defs.h | 18 ------------------
> > > > 2 files changed, 23 insertions(+), 30 deletions(-)
> > > >
> > > > diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
> > > > index fa0fa30bb9..49df7b7d54 100644
> > > > --- a/hw/acpi/pci.c
> > > > +++ b/hw/acpi/pci.c
> > > > @@ -30,17 +30,28 @@
> > > >
> > > > void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo
> > > > *info)
> > > > {
> > > > - AcpiTableMcfg *mcfg;
> > > > - int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]);
> > > > -
> > > > - mcfg = acpi_data_push(table_data, len);
> > > > - mcfg->allocation[0].address = cpu_to_le64(info->base);
> > > > -
> > > > - /* Only a single allocation so no need to play with segments */
> > > > - mcfg->allocation[0].pci_segment = cpu_to_le16(0);
> > > > - mcfg->allocation[0].start_bus_number = 0;
> > > > - mcfg->allocation[0].end_bus_number = PCIE_MMCFG_BUS(info->size -
> > > > 1);
> > > > -
> > > > - build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1,
> > > > NULL, NULL);
> > > > + int mcfg_start = table_data->len;
> > > > +
> > > > + acpi_data_push(table_data, sizeof(AcpiTableHeader));
> > > > +
> > > > + /*
> > > > + * PCI Firmware Specification, Revision 3.0
> > > > + * 4.1.2 MCFG Table Description.
> > > > + */
> > > > + /* Reserved */
> > > > + build_append_int_noprefix(table_data, 0, 8);
> > > > + /* Base address, processor-relative */
> > > > + build_append_int_noprefix(table_data, info->base, 8);
> > > > + /* PCI segment group number */
> > > > + build_append_int_noprefix(table_data, 0, 2);
> > > > + /* Starting PCI Bus number */
> > > > + build_append_int_noprefix(table_data, 0, 1);
> > > > + /* Final PCI Bus number */
> > > > + build_append_int_noprefix(table_data, PCIE_MMCFG_BUS(info->size -
> > > > 1), 1);
> > > > + /* Reserved */
> > > > + build_append_int_noprefix(table_data, 0, 4);
> > > > +
> > > > + build_header(linker, table_data, (void *)(table_data->data +
> > > > mcfg_start),
> > > > + "MCFG", table_data->len - mcfg_start, 1, NULL, NULL);
> > > > }
> > > >
> > > > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> > > > index f9aa4bd398..57a3f58b0c 100644
> > > > --- a/include/hw/acpi/acpi-defs.h
> > > > +++ b/include/hw/acpi/acpi-defs.h
> > > > @@ -449,24 +449,6 @@ struct AcpiSratProcessorGiccAffinity {
> > > >
> > > > typedef struct AcpiSratProcessorGiccAffinity
> > > > AcpiSratProcessorGiccAffinity;
> > > >
> > > > -/* PCI fw r3.0 MCFG table. */
> > > > -/* Subtable */
> > > > -struct AcpiMcfgAllocation {
> > > > - uint64_t address; /* Base address,
> > > > processor-relative */
> > > > - uint16_t pci_segment; /* PCI segment group number */
> > > > - uint8_t start_bus_number; /* Starting PCI Bus number */
> > > > - uint8_t end_bus_number; /* Final PCI Bus number */
> > > > - uint32_t reserved;
> > > > -} QEMU_PACKED;
> > > > -typedef struct AcpiMcfgAllocation AcpiMcfgAllocation;
> > > > -
> > > > -struct AcpiTableMcfg {
> > > > - ACPI_TABLE_HEADER_DEF;
> > > > - uint8_t reserved[8];
> > > > - AcpiMcfgAllocation allocation[0];
> > > > -} QEMU_PACKED;
> > > > -typedef struct AcpiTableMcfg AcpiTableMcfg;
> > > > -
> > > > /*
> > > > * TCPA Description Table
> > > > *
> > > > --
> > > > 2.19.1
