Re: [PATCH v5 07/10] ACPI ERST: trace support

[Igor Mammedov]

On Wed, 21 Jul 2021 11:14:37 -0500
Eric DeVolder <eric.devolder@oracle.com> wrote:

> On 7/20/21 8:15 AM, Igor Mammedov wrote:
> > On Wed, 30 Jun 2021 15:07:18 -0400
> > Eric DeVolder <eric.devolder@oracle.com> wrote:
> >   
> >> Provide the definitions needed to support tracing in ACPI ERST.  
> > trace points should be introduced in patches that use them for the first 
> > time,
> > as it stands now series breaks bisection.  
> 
> Are you asking to move this patch before the patch that introduces erst.c 
> (which
> uses these trace points)?
> Or are you asking to include this patch with the patch that introduces erst.c?

I'd squash it into patch that introduces corresponding functions.

> Also, you requested I separate the building of ERST table from the 
> implemenation
> of the erst device as separate patches. Doesn't that also break bisection?

it should be possible to compile series patch by patch and not break 'make 
check'
after each patch.

ACPI table is not part of device, it's separate part that describes to OSPM
how to work with device. So if code split correctly between patches
it shouldn't break bisection.

> 
> >   
> >>
> >> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> >> ---
> >>   hw/acpi/trace-events | 14 ++++++++++++++
> >>   1 file changed, 14 insertions(+)
> >>
> >> diff --git a/hw/acpi/trace-events b/hw/acpi/trace-events
> >> index dcc1438..a5c2755 100644
> >> --- a/hw/acpi/trace-events
> >> +++ b/hw/acpi/trace-events
> >> @@ -55,3 +55,17 @@ piix4_gpe_writeb(uint64_t addr, unsigned width, 
> >> uint64_t val) "addr: 0x%" PRIx64
> >>   # tco.c
> >>   tco_timer_reload(int ticks, int msec) "ticks=%d (%d ms)"
> >>   tco_timer_expired(int timeouts_no, bool strap, bool no_reboot) 
> >> "timeouts_no=%d no_reboot=%d/%d"
> >> +
> >> +# erst.c
> >> +acpi_erst_reg_write(uint64_t addr, uint64_t val, unsigned size) "addr: 
> >> 0x%04" PRIx64 " <== 0x%016" PRIx64 " (size: %u)"
> >> +acpi_erst_reg_read(uint64_t addr, uint64_t val, unsigned size) " addr: 
> >> 0x%04" PRIx64 " ==> 0x%016" PRIx64 " (size: %u)"
> >> +acpi_erst_mem_write(uint64_t addr, uint64_t val, unsigned size) "addr: 
> >> 0x%06" PRIx64 " <== 0x%016" PRIx64 " (size: %u)"
> >> +acpi_erst_mem_read(uint64_t addr, uint64_t val, unsigned size) " addr: 
> >> 0x%06" PRIx64 " ==> 0x%016" PRIx64 " (size: %u)"
> >> +acpi_erst_pci_bar_0(uint64_t addr) "BAR0: 0x%016" PRIx64
> >> +acpi_erst_pci_bar_1(uint64_t addr) "BAR1: 0x%016" PRIx64
> >> +acpi_erst_realizefn_in(void)
> >> +acpi_erst_realizefn_out(unsigned size) "total nvram size %u bytes"
> >> +acpi_erst_reset_in(unsigned record_count) "record_count %u"
> >> +acpi_erst_reset_out(unsigned record_count) "record_count %u"
> >> +acpi_erst_class_init_in(void)
> >> +acpi_erst_class_init_out(void)  
> >   
>