Re: [PATCH v22 7/9] ACPI: Record Generic Error Status Block(GESB) table

From: Igor Mammedov <imammedo@redhat.com>
To: gengdongjiu <gengdongjiu@huawei.com>
Cc: <pbonzini@redhat.com>, <mst@redhat.com>,
	<shannon.zhaosl@gmail.com>, <peter.maydell@linaro.org>,
	<fam@euphon.net>, <rth@twiddle.net>, <ehabkost@redhat.com>,
	<mtosatti@redhat.com>, <xuwei5@huawei.com>,
	<jonathan.cameron@huawei.com>, <james.morse@arm.com>,
	<qemu-devel@nongnu.org>, <kvm@vger.kernel.org>,
	<qemu-arm@nongnu.org>, <zhengxiang9@huawei.com>,
	<linuxarm@huawei.com>
Subject: Re: [PATCH v22 7/9] ACPI: Record Generic Error Status Block(GESB) table
Date: Mon, 3 Feb 2020 08:55:01 +0100	[thread overview]
Message-ID: <20200203085501.499e3a83@redhat.com> (raw)
In-Reply-To: <a9f46632-0766-7e82-7dc4-752d00b4a0a1@huawei.com>

On Sun, 2 Feb 2020 21:42:23 +0800
gengdongjiu <gengdongjiu@huawei.com> wrote:

> On 2020/1/28 23:29, Igor Mammedov wrote:
> > On Wed, 8 Jan 2020 19:32:21 +0800
> > Dongjiu Geng <gengdongjiu@huawei.com> wrote:
> >   
> >> kvm_arch_on_sigbus_vcpu() error injection uses source_id as
> >> index in etc/hardware_errors to find out Error Status Data
> >> Block entry corresponding to error source. So supported source_id
> >> values should be assigned here and not be changed afterwards to
> >> make sure that guest will write error into expected Error Status
> >> Data Block even if guest was migrated to a newer QEMU.
> >>
> >> Before QEMU writes a new error to ACPI table, it will check whether
> >> previous error has been acknowledged. Otherwise it will ignore the new
> >> error. For the errors section type, QEMU simulate it to memory section
> >> error.
> >>
> >> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
> >> Signed-off-by: Xiang Zheng <zhengxiang9@huawei.com>
> >> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

btw:
when you are changing patch and it's not a trivial change,
you are supposed to drop Reviewed-by/Acked-by tags.

[...]
> >> diff --git a/include/qemu/uuid.h b/include/qemu/uuid.h
> >> index 129c45f..b35e294 100644
> >> --- a/include/qemu/uuid.h
> >> +++ b/include/qemu/uuid.h
> >> @@ -34,6 +34,11 @@ typedef struct {
> >>      };
> >>  } QemuUUID;
> >>  
> >> +#define UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)             \
> >> +  {{{ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \
> >> +     (b) & 0xff, ((b) >> 8) & 0xff, (c) & 0xff, ((c) >> 8) & 0xff,          \
> >> +     (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) } } }  
> > 
> > since you are adding generalizing macro, take of NVDIMM_UUID_LE which served as model  
> 
> do you mean use this generalizing macro to replace NVDIMM_UUID_LE, right?

yes, and preferably do that in a separate patch

> 
> > 
> >   
> >>  #define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-" \
> >>                   "%02hhx%02hhx-%02hhx%02hhx-" \
> >>                   "%02hhx%02hhx-" \  
> > 
> > .
> >   
> 