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

[Eric DeVolder <eric.devolder@oracle.com>]

On 7/26/21 6:08 AM, Igor Mammedov wrote:
> 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.
Done

> 
>> 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.
<nods>

> 
>>
>>>    
>>>>
>>>> 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)
>>>    
>>
>