QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Dongjiu Geng <gengdongjiu@huawei.com>
To: Peter Maydell <peter.maydell@linaro.org>,
	"Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>,
	QEMU Developers <qemu-devel@nongnu.org>
Subject: Re: [PULL 26/45] ACPI: Record Generic Error Status Block(GESB) table
Date: Sat, 20 Jun 2020 09:50:23 +0800
Message-ID: <ca79ea28-9ea9-18a5-99ad-25c3eb744721@huawei.com> (raw)
In-Reply-To: <CAFEAcA-PTtLvouxo5XZmgSbeRWa4WCwH7_cC5xrg3Dnr8UyZxg@mail.gmail.com>



On 2020/6/20 1:21, Peter Maydell wrote:
> On Thu, 21 May 2020 at 16:31, Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>> On Thu, May 21, 2020 at 02:03:36PM +0100, Peter Maydell wrote:
>>> On Thu, 14 May 2020 at 15:22, Peter Maydell <peter.maydell@linaro.org> wrote:
>>>>
>>>> From: Dongjiu Geng <gengdongjiu@huawei.com>
>>>>
>>>> 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.
>>>>
>>>> Before QEMU writes a new error to ACPI table, it will check whether
>>>> previous error has been acknowledged. If not acknowledged, the new
>>>> errors will be ignored and not be recorded. For the errors section
>>>> type, QEMU simulate it to memory section error.
>>>
>>> Hi; Coverity points out (CID 1428962) that there is
>>> unreachable code in this function:
>>>
>>>> +static int acpi_ghes_record_mem_error(uint64_t error_block_address,
>>>> +                                      uint64_t error_physical_addr)
>>>> +{
>>>> +    GArray *block;
>>>> +
>>>> +    /* Memory Error Section Type */
>>>> +    const uint8_t uefi_cper_mem_sec[] =
>>>> +          UUID_LE(0xA5BC1114, 0x6F64, 0x4EDE, 0xB8, 0x63, 0x3E, 0x83, \
>>>> +                  0xED, 0x7C, 0x83, 0xB1);
>>>> +
>>>> +    /* invalid fru id: ACPI 4.0: 17.3.2.6.1 Generic Error Data,
>>>> +     * Table 17-13 Generic Error Data Entry
>>>> +     */
>>>> +    QemuUUID fru_id = {};
>>>> +    uint32_t data_length;
>>>> +
>>>> +    block = g_array_new(false, true /* clear */, 1);
>>>> +
>>>> +    /* This is the length if adding a new generic error data entry*/
>>>> +    data_length = ACPI_GHES_DATA_LENGTH + ACPI_GHES_MEM_CPER_LENGTH;
>>>
>>> Here data_length has a constant value...
>>>
>>>> +
>>>> +    /*
>>>> +     * Check whether it will run out of the preallocated memory if adding a new
>>>> +     * generic error data entry
>>>> +     */
>>>> +    if ((data_length + ACPI_GHES_GESB_SIZE) > ACPI_GHES_MAX_RAW_DATA_LENGTH) {
>>>
>>> ...but here we immediately have a runtime check which can't possibly
>>> fail because of the values of the constants involved, so this
>>> if() block is dead code.
>>>
>>>> +        error_report("Not enough memory to record new CPER!!!");
>>>> +        g_array_free(block, true);
>>>> +        return -1;
>>>> +    }
>>>
>>> What was this code trying to do? Is the initial value of
>>> data_length incorrect, or is the if() condition wrong, or
>>> should this simply have been an assert() ?
> 
>> It's just a validity check. assert will do just as well.
> 
> Would somebody like to write a patch to make it assert instead, then,
> please? That should keep Coverity happy.
  I will check the comments history and make a patch, thanks a lot.

> 
> thanks
> -- PMM
> 
> .
> 



  reply index

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-14 14:20 [PULL 00/45] target-arm queue Peter Maydell
2020-05-14 14:20 ` [PULL 01/45] target/arm: Use correct GDB XML for M-profile cores Peter Maydell
2020-05-14 14:20 ` [PULL 02/45] target/arm: Create gen_gvec_[us]sra Peter Maydell
2020-05-14 14:20 ` [PULL 03/45] target/arm: Create gen_gvec_{u,s}{rshr,rsra} Peter Maydell
2020-05-14 14:20 ` [PULL 04/45] target/arm: Create gen_gvec_{sri,sli} Peter Maydell
2020-05-14 14:20 ` [PULL 05/45] target/arm: Remove unnecessary range check for VSHL Peter Maydell
2020-05-14 14:20 ` [PULL 06/45] target/arm: Tidy handle_vec_simd_shri Peter Maydell
2020-05-14 14:21 ` [PULL 07/45] target/arm: Create gen_gvec_{ceq,clt,cle,cgt,cge}0 Peter Maydell
2020-05-14 14:21 ` [PULL 08/45] target/arm: Create gen_gvec_{mla,mls} Peter Maydell
2020-05-14 14:21 ` [PULL 09/45] target/arm: Swap argument order for VSHL during decode Peter Maydell
2020-05-14 14:21 ` [PULL 10/45] target/arm: Create gen_gvec_{cmtst,ushl,sshl} Peter Maydell
2020-05-14 14:21 ` [PULL 11/45] target/arm: Create gen_gvec_{uqadd, sqadd, uqsub, sqsub} Peter Maydell
2020-05-14 14:21 ` [PULL 12/45] target/arm: Remove fp_status from helper_{recpe, rsqrte}_u32 Peter Maydell
2020-05-14 14:21 ` [PULL 13/45] target/arm: Create gen_gvec_{qrdmla,qrdmls} Peter Maydell
2020-05-14 14:21 ` [PULL 14/45] target/arm: Pass pointer to qc to qrdmla/qrdmls Peter Maydell
2020-05-14 14:21 ` [PULL 15/45] target/arm: Clear tail in gvec_fmul_idx_*, gvec_fmla_idx_* Peter Maydell
2020-05-14 14:21 ` [PULL 16/45] target/arm: Vectorize SABD/UABD Peter Maydell
2020-05-14 14:21 ` [PULL 17/45] target/arm: Vectorize SABA/UABA Peter Maydell
2020-05-21 13:11   ` Peter Maydell
2020-05-14 14:21 ` [PULL 18/45] aspeed: Add support for the sonorapass-bmc board Peter Maydell
2020-05-14 14:21 ` [PULL 19/45] acpi: nvdimm: change NVDIMM_UUID_LE to a common macro Peter Maydell
2020-05-14 14:21 ` [PULL 20/45] hw/arm/virt: Introduce a RAS machine option Peter Maydell
2020-05-14 14:21 ` [PULL 21/45] docs: APEI GHES generation and CPER record description Peter Maydell
2020-05-14 14:21 ` [PULL 22/45] ACPI: Build related register address fields via hardware error fw_cfg blob Peter Maydell
2020-05-14 14:21 ` [PULL 23/45] ACPI: Build Hardware Error Source Table Peter Maydell
2020-05-14 14:21 ` [PULL 24/45] ACPI: Record the Generic Error Status Block address Peter Maydell
2020-05-14 14:21 ` [PULL 25/45] KVM: Move hwpoison page related functions into kvm-all.c Peter Maydell
2020-05-14 14:21 ` [PULL 26/45] ACPI: Record Generic Error Status Block(GESB) table Peter Maydell
2020-05-21 13:03   ` Peter Maydell
2020-05-21 15:31     ` Michael S. Tsirkin
2020-06-19 17:21       ` Peter Maydell
2020-06-20  1:50         ` Dongjiu Geng [this message]
2020-05-14 14:21 ` [PULL 27/45] target-arm: kvm64: handle SIGBUS signal from kernel or KVM Peter Maydell
2020-05-14 14:21 ` [PULL 28/45] MAINTAINERS: Add ACPI/HEST/GHES entries Peter Maydell
2020-05-14 14:21 ` [PULL 29/45] target/arm: Convert Neon 3-reg-same VQRDMLAH/VQRDMLSH to decodetree Peter Maydell
2020-05-14 14:21 ` [PULL 30/45] target/arm: Convert Neon 3-reg-same SHA " Peter Maydell
2020-05-14 14:21 ` [PULL 31/45] target/arm: Convert Neon 64-bit element 3-reg-same insns Peter Maydell
2020-05-14 14:21 ` [PULL 32/45] target/arm: Convert Neon VHADD " Peter Maydell
2020-05-14 14:21 ` [PULL 33/45] target/arm: Convert Neon VABA/VABD 3-reg-same to decodetree Peter Maydell
2020-05-14 14:21 ` [PULL 34/45] target/arm: Convert Neon VRHADD, VHSUB 3-reg-same insns " Peter Maydell
2020-05-14 14:21 ` [PULL 35/45] target/arm: Convert Neon VQSHL, VRSHL, VQRSHL " Peter Maydell
2020-05-14 14:21 ` [PULL 36/45] target/arm: Convert Neon VPMAX/VPMIN " Peter Maydell
2020-05-14 14:21 ` [PULL 37/45] target/arm: Convert Neon VPADD " Peter Maydell
2020-05-14 14:21 ` [PULL 38/45] target/arm: Convert Neon VQDMULH/VQRDMULH 3-reg-same " Peter Maydell
2020-05-14 14:21 ` [PULL 39/45] target/arm: Convert Neon VADD, VSUB, VABD 3-reg-same insns " Peter Maydell
2020-05-14 14:21 ` [PULL 40/45] target/arm: Convert Neon VPMIN/VPMAX/VPADD float " Peter Maydell
2020-05-14 14:21 ` [PULL 41/45] target/arm: Convert Neon fp VMUL, VMLA, VMLS " Peter Maydell
2020-05-14 14:21 ` [PULL 42/45] target/arm: Convert Neon 3-reg-same compare " Peter Maydell
2020-05-14 14:21 ` [PULL 43/45] target/arm: Move 'env' argument of recps_f32 and rsqrts_f32 helpers to usual place Peter Maydell
2020-05-14 14:21 ` [PULL 44/45] target/arm: Convert Neon fp VMAX/VMIN/VMAXNM/VMINNM/VRECPS/VRSQRTS to decodetree Peter Maydell
2020-05-14 14:21 ` [PULL 45/45] target/arm: Convert NEON VFMA, VFMS 3-reg-same insns " Peter Maydell
2020-05-14 16:42 ` [PULL 00/45] target-arm queue Peter Maydell

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=ca79ea28-9ea9-18a5-99ad-25c3eb744721@huawei.com \
    --to=gengdongjiu@huawei.com \
    --cc=imammedo@redhat.com \
    --cc=mst@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /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

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git
	git clone --mirror https://lore.kernel.org/qemu-devel/2 qemu-devel/git/2.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org
	public-inbox-index qemu-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git