linux-kernel - Re: [PATCH v11 5/6] target-arm: kvm64: handle SIGBUS signal for synchronous External Abort

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAFEAcA8HPhHeeXEDtxnewJ7mAJ-gYzR96pC06qA+Xx95LHu2DA@mail.gmail.com>
Date:   Fri, 8 Sep 2017 17:21:52 +0100
From:   Peter Maydell <peter.maydell@...aro.org>
To:     gengdongjiu <gengdongjiu@...wei.com>
Cc:     "Michael S. Tsirkin" <mst@...hat.com>,
        Igor Mammedov <imammedo@...hat.com>,
        Zhaoshenglong <zhaoshenglong@...wei.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        QEMU Developers <qemu-devel@...gnu.org>,
        qemu-arm <qemu-arm@...gnu.org>, kvm-devel <kvm@...r.kernel.org>,
        "edk2-devel@...ts.01.org" <edk2-devel@...ts.01.org>,
        Christoffer Dall <christoffer.dall@...aro.org>,
        Marc Zyngier <marc.zyngier@....com>,
        Will Deacon <will.deacon@....com>,
        James Morse <james.morse@....com>,
        Tyler Baicar <tbaicar@...eaurora.org>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Ingo Molnar <mingo@...nel.org>, "bp@...e.de" <bp@...e.de>,
        Shiju Jose <shiju.jose@...wei.com>,
        "zjzhang@...eaurora.org" <zjzhang@...eaurora.org>,
        arm-mail-list <linux-arm-kernel@...ts.infradead.org>,
        "kvmarm@...ts.cs.columbia.edu" <kvmarm@...ts.cs.columbia.edu>,
        lkml - Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
        "devel@...ica.org" <devel@...ica.org>,
        John Garry <john.garry@...wei.com>,
        Jonathan Cameron <jonathan.cameron@...wei.com>,
        Shameerali Kolothum Thodi 
        <shameerali.kolothum.thodi@...wei.com>,
        huangdaode <huangdaode@...ilicon.com>,
        "Wangzhou (B)" <wangzhou1@...ilicon.com>,
        Huangshaoyu <huangshaoyu@...wei.com>,
        Wuquanming <wuquanming@...wei.com>,
        Linuxarm <linuxarm@...wei.com>,
        "Zhengqiang (turing)" <zhengqiang10@...wei.com>
Subject: Re: [PATCH v11 5/6] target-arm: kvm64: handle SIGBUS signal for
 synchronous External Abort

On 8 September 2017 at 17:17, gengdongjiu <gengdongjiu@...wei.com> wrote:
>>
>> This code has all just been copied-and-pasted from target/i386/kvm.c.
>> Please instead abstract it out properly into a cpu-independent source file.
>
>
> Yes, it copied from x86.
> Do you mean abstracting this code to a common folder so that i386 and arm platform share it?

I mean it should go into a common source file (perhaps
accel/kvm/kvm-all.c).


>> What is this ??? You should never need to look up things in the cpreg arrays by fieldoffset.
>
>
> I used it to set the esr_el1's and far_el1's value, for example:
>
>   /* set ESR_EL1 */
>   ret = kvm_arm_cpreg_value(cpu, offsetof(CPUARMState, cp15.esr_el[1]));
>
>   /* set FAR_EL1 */
>   ret = kvm_arm_cpreg_value(cpu, offsetof(CPUARMState, cp15.far_el[1]));

Yes, I saw that, but I have no idea why you think that's
the right way to set register values. No other code in
QEMU does this.

> So use the added API kvm_arm_cpreg_value() to set their value.
> If not used it, do you better method to set their value?

I suggest:

>> The code for handling debug exits (software step, watchpoint,
>> etc) is probably a good place to look for how to deal with register state.

>> Any new globally-visible function prototype in a header should
>> have a doc-comment formatted documentation comment, please.
>
>
> Ok, thanks for this reminder. Do you mean I need to add comments
> for this globally-visible function, such as below:
>
> /*
>  * xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
>  */
> void kvm_hwpoison_page_add(ram_addr_t ram_addr);

It should be in the doc-comment format, which begins
"/**" and has some stylization of how you list parameters
and so on. Lots of examples in the existing headers.

thanks
-- PMM