* Re: [PATCH v9 3/7] acpi: apei: Add SEI notification type support for ARMv8 @ 2018-02-05 11:24 gengdongjiu 2018-02-15 17:55 ` James Morse 0 siblings, 1 reply; 10+ messages in thread From: gengdongjiu @ 2018-02-05 11:24 UTC (permalink / raw) To: James Morse Cc: christoffer.dall, marc.zyngier, linux, catalin.marinas, rjw, bp, robert.moore, lv.zheng, corbet, will.deacon, linux-doc, linux-kernel, linux-arm-kernel, kvmarm, linux-acpi, devel, Huangshaoyu, Liujun (Jun Liu) [...] > > > Yes, I know you are dong that. Your serial's patch will consider all above > things, right? > > Assuming I got it right, yes. It currently makes the race Xie XiuQi spotted worse, > which I want to fix too. (details on the cover letter) Ok. > > > > If your patch can be consider that, this patch can based on your patchset. > thanks. > > I'd like to pick these patches onto the end of that series, but first I want to > know what NOTIFY_SEI means for any OS. The ACPI spec doesn't say, and > because its asynchronous, route-able and mask-able, there are many more > corners than NOTFIY_SEA. > > This thing is a notification using an emulated SError exception. (emulated > because physical-SError must be routed to EL3 for firmware-first, and > virtual-SError belongs to EL2). > > Does your firmware emulate SError exactly as the TakeException() pseudo code > in the Arm-Arm? Yes, it is. > Is the emulated SError routed following the routing rules for HCR_EL2.{AMO, > TGE}? Yes, it is. > What does your firmware do when it wants to emulate SError but its masked? > (e.g.1: The physical-SError interrupted EL2 and the SPSR shows EL2 had > PSTATE.A set. > e.g.2: The physical-SError interrupted EL2 but HCR_EL2 indicates the > emulated SError should go to EL1. This effectively masks SError.) Currently we does not consider much about the mask status(SPSR). I remember that you ever suggested firmware should reboot if the mask status is set(SPSR), right? I ever suggest our firmware team to evaluate that, but there is no response. I CC "liu jun" <liujun88@hisilicon.com> who is our UEFI firmware Architect, if you have firmware requirements, you can raise again. > > Answers to these let us determine whether a bug is in the firmware or the > kernel. If firmware is expecting the OS to do something special, I'd like to know > about it from the beginning! I know your meaning, thanks for raising it again. > > > >>> Expose API ghes_notify_sei() to external users. External modules can > >>> call this exposed API to parse APEI table and handle the SEI > >>> notification. > >> > >> external modules? You mean called by the arch code when it gets this > NOTIFY_SEI? > > > yes, called by kernel ARCH code, such as below, I remember I have discussed > with you. > > Sure. The phrase 'external modules' usually means the '.ko' files that live in > /lib/modules, nothing outside the kernel tree should be doing this stuff. I will rename 'external modules' to other name. Thanks. > > > Thanks, > > James ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v9 3/7] acpi: apei: Add SEI notification type support for ARMv8 2018-02-05 11:24 [PATCH v9 3/7] acpi: apei: Add SEI notification type support for ARMv8 gengdongjiu @ 2018-02-15 17:55 ` James Morse 2018-04-12 5:00 ` gengdongjiu 0 siblings, 1 reply; 10+ messages in thread From: James Morse @ 2018-02-15 17:55 UTC (permalink / raw) To: gengdongjiu Cc: christoffer.dall, marc.zyngier, linux, catalin.marinas, rjw, bp, robert.moore, lv.zheng, corbet, will.deacon, linux-doc, linux-kernel, linux-arm-kernel, kvmarm, linux-acpi, devel, Huangshaoyu, Liujun (Jun Liu) Hi gengdongjiu, liu jun On 05/02/18 11:24, gengdongjiu wrote: > James Morse wrote: >> I'd like to pick these patches onto the end of that series, but first I want to >> know what NOTIFY_SEI means for any OS. The ACPI spec doesn't say, and >> because its asynchronous, route-able and mask-able, there are many more >> corners than NOTFIY_SEA. >> >> This thing is a notification using an emulated SError exception. (emulated >> because physical-SError must be routed to EL3 for firmware-first, and >> virtual-SError belongs to EL2). >> >> Does your firmware emulate SError exactly as the TakeException() pseudo code >> in the Arm-Arm? > > Yes, it is. > >> Is the emulated SError routed following the routing rules for HCR_EL2.{AMO, >> TGE}? > > Yes, it is. ... and yet ... >> What does your firmware do when it wants to emulate SError but its masked? >> (e.g.1: The physical-SError interrupted EL2 and the SPSR shows EL2 had >> PSTATE.A set. >> e.g.2: The physical-SError interrupted EL2 but HCR_EL2 indicates the >> emulated SError should go to EL1. This effectively masks SError.) > > Currently we does not consider much about the mask status(SPSR). .. this is a problem. If you ignore SPSR_EL3 you may deliver an SError to EL1 when the exception interrupted EL2. Even if you setup the EL1 register correctly, EL1 can't eret to EL2. This should never happen, SError is effectively masked if you are running at an EL higher than the one its routed to. More obviously: if the exception came from the EL that SError should be routed to, but PSTATE.A was set, you can't deliver SError. Masking SError is the only way the OS has to indicate it can't take an exception right now. VBAR_EL1 may be 'wrong' if we're doing some power-management, the FAR/ELR/ESR/SPSR registers may contain live values that the OS would lose if you deliver another exception over the top. If you deliver an emulated-SError as the OS eret's, your new ELR will point at the eret instruction and the CPU will spin on this instruction forever. You have to honour the masking and routing rules for SError, otherwise no OS can run safely with this firmware. > I remember that you ever suggested firmware should reboot if the mask status > is set(SPSR), right? Yes, this is my suggestion of what to do if you can't deliver an SError: store the RAS error in the BERT and 'reboot'. > I CC "liu jun" <liujun88@hisilicon.com> who is our UEFI firmware Architect, > if you have firmware requirements, you can raise again. (UEFI? I didn't think there was any of that at EL3, but I'm not familiar with all the 'PI' bits). The requirement is your emulated-SError from EL3 looks exactly like a physical-SError as if EL3 wasn't implemented. Your CPU has to handle cases where it can't deliver an SError, your emulation has to do the same. This is not something any OS can work around. >> Answers to these let us determine whether a bug is in the firmware or the >> kernel. If firmware is expecting the OS to do something special, I'd like to know >> about it from the beginning! > > I know your meaning, thanks for raising it again. Happy new year, James ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v9 3/7] acpi: apei: Add SEI notification type support for ARMv8 2018-02-15 17:55 ` James Morse @ 2018-04-12 5:00 ` gengdongjiu 2018-04-12 16:14 ` James Morse 0 siblings, 1 reply; 10+ messages in thread From: gengdongjiu @ 2018-04-12 5:00 UTC (permalink / raw) To: James Morse, lishuo1, merry.libing Cc: gengdongjiu, linux-arm-kernel, Liujun (Jun Liu), linux-kernel, corbet, marc.zyngier, catalin.marinas, linux-doc, rjw, linux, will.deacon, robert.moore, linux-acpi, bp, lv.zheng, Huangshaoyu, kvmarm, devel Dear James, Thanks for this mail and sorry for my late response. 2018-02-16 1:55 GMT+08:00 James Morse <james.morse@arm.com>: > Hi gengdongjiu, liu jun > > On 05/02/18 11:24, gengdongjiu wrote: [....] >> >>> Is the emulated SError routed following the routing rules for HCR_EL2.{AMO, >>> TGE}? >> >> Yes, it is. > > ... and yet ... > > >>> What does your firmware do when it wants to emulate SError but its masked? >>> (e.g.1: The physical-SError interrupted EL2 and the SPSR shows EL2 had >>> PSTATE.A set. >>> e.g.2: The physical-SError interrupted EL2 but HCR_EL2 indicates the >>> emulated SError should go to EL1. This effectively masks SError.) >> >> Currently we does not consider much about the mask status(SPSR). > > .. this is a problem. > > If you ignore SPSR_EL3 you may deliver an SError to EL1 when the exception > interrupted EL2. Even if you setup the EL1 register correctly, EL1 can't eret to > EL2. This should never happen, SError is effectively masked if you are running > at an EL higher than the one its routed to. > > More obviously: if the exception came from the EL that SError should be routed > to, but PSTATE.A was set, you can't deliver SError. Masking SError is the only James, I summarized the masking and routing rules for SError to confirm with you for the firmware first solution, 1. If the HCR_EL2.{AMO,TGE} is set, which means the SError should route to EL2, When system happens SError and trap to EL3, If EL3 find HCR_EL2.{AMO,TGE} and SPSR_EL3.A are both set, and find this SError come from EL2, it will not deliver an SError: store the RAS error in the BERT and 'reboot'; but if it find that this SError come from EL1 or EL0, it also need to deliver an SError, right? 2. If the HCR_EL2.{AMO,TGE} is not set, which means the SError should route to EL1, When system happens SError and trap to EL3, If EL3 find HCR_EL2.{AMO,TGE} and SPSR_EL3.A are both not set, and find this SError come from EL1, it will not deliver an SError: store the RAS error in the BERT and 'reboot'; but if it find that this SError come from EL0, it also need to deliver an SError, right? > way the OS has to indicate it can't take an exception right now. VBAR_EL1 may be > 'wrong' if we're doing some power-management, the FAR/ELR/ESR/SPSR registers may > contain live values that the OS would lose if you deliver another exception over > the top. > > If you deliver an emulated-SError as the OS eret's, your new ELR will point at > the eret instruction and the CPU will spin on this instruction forever. > > You have to honour the masking and routing rules for SError, otherwise no OS can > run safely with this firmware. > > >> I remember that you ever suggested firmware should reboot if the mask status >> is set(SPSR), right? > > Yes, this is my suggestion of what to do if you can't deliver an SError: store > the RAS error in the BERT and 'reboot'. > > >> I CC "liu jun" <liujun88@hisilicon.com> who is our UEFI firmware Architect, >> if you have firmware requirements, you can raise again. > > (UEFI? I didn't think there was any of that at EL3, but I'm not familiar with > all the 'PI' bits). > > The requirement is your emulated-SError from EL3 looks exactly like a > physical-SError as if EL3 wasn't implemented. > Your CPU has to handle cases where it can't deliver an SError, your emulation > has to do the same. > > This is not something any OS can work around. > > >>> Answers to these let us determine whether a bug is in the firmware or the >>> kernel. If firmware is expecting the OS to do something special, I'd like to know >>> about it from the beginning! >> >> I know your meaning, thanks for raising it again. > > > Happy new year, > > James > _______________________________________________ > kvmarm mailing list > kvmarm@lists.cs.columbia.edu > https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v9 3/7] acpi: apei: Add SEI notification type support for ARMv8 2018-04-12 5:00 ` gengdongjiu @ 2018-04-12 16:14 ` James Morse 2018-04-13 13:50 ` gengdongjiu 0 siblings, 1 reply; 10+ messages in thread From: James Morse @ 2018-04-12 16:14 UTC (permalink / raw) To: gengdongjiu Cc: lishuo1, merry.libing, gengdongjiu, linux-arm-kernel, Liujun (Jun Liu), linux-kernel, corbet, marc.zyngier, catalin.marinas, linux-doc, rjw, linux, will.deacon, robert.moore, linux-acpi, bp, lv.zheng, Huangshaoyu, kvmarm, devel Hi gengdongjiu, On 12/04/18 06:00, gengdongjiu wrote: > 2018-02-16 1:55 GMT+08:00 James Morse <james.morse@arm.com>: >> On 05/02/18 11:24, gengdongjiu wrote: >>>> Is the emulated SError routed following the routing rules for HCR_EL2.{AMO, >>>> TGE}? >>> >>> Yes, it is. >> >> ... and yet ... >> >> >>>> What does your firmware do when it wants to emulate SError but its masked? >>>> (e.g.1: The physical-SError interrupted EL2 and the SPSR shows EL2 had >>>> PSTATE.A set. >>>> e.g.2: The physical-SError interrupted EL2 but HCR_EL2 indicates the >>>> emulated SError should go to EL1. This effectively masks SError.) >>> >>> Currently we does not consider much about the mask status(SPSR). >> >> .. this is a problem. >> >> If you ignore SPSR_EL3 you may deliver an SError to EL1 when the exception >> interrupted EL2. Even if you setup the EL1 register correctly, EL1 can't eret to >> EL2. This should never happen, SError is effectively masked if you are running >> at an EL higher than the one its routed to. >> >> More obviously: if the exception came from the EL that SError should be routed >> to, but PSTATE.A was set, you can't deliver SError. Masking SError is the only > James, I summarized the masking and routing rules for SError to > confirm with you for the firmware first solution, You also said "Currently we does not consider much about the mask status(SPSR)." > 1. If the HCR_EL2.{AMO,TGE} is set, If one or the other of these bits is set: (AMO==1 || TGE==1) > which means the SError should route to EL2, > When system happens SError and trap to EL3, If EL3 find > HCR_EL2.{AMO,TGE} and SPSR_EL3.A are both set, > and find this SError come from EL2, it will not deliver an SError: > store the RAS error in the BERT and 'reboot'; but if > it find that this SError come from EL1 or EL0, it also need to deliver > an SError, right? Yes. > 2. If the HCR_EL2.{AMO,TGE} is not set, If neither of these bits is set: (AMO==0 && TGE == 0) > which means the SError should route to EL1, > When system happens SError and trap to EL3, If EL3 find > HCR_EL2.{AMO,TGE} and SPSR_EL3.A are both not set, (I'm reading this as all three of these bits are clear) > and find this SError come from EL1, it will not deliver an SError: > store the RAS error in the BERT and 'reboot'; No, (AMO==0 && TGE == 0) means SError is routed to EL1, this exception interrupted EL1 and the A bit was clear, so EL1 can take an SError. The two cases here are: AMO==0,TGE==0 means SError should be routed to EL1. If SPSR_EL3 says the exception interrupted EL1 and the A bit was set, you need to do the BERT trick. If SPSR_EL3 says the exception interrupted EL2, you need to do the BERT trick regardless of the A bit, as SError is implicitly masked by running at a higher exception level than it was routed to. >From your v11 reply: > 2. The exception came from the EL that SError should not be routed > to(according to hcr_EL2.{AMO, TGE}),even though the PSTATE.A was set,EL3 > firmware still deliver SError (this is re-iterating the two-cases above:) 'not be routed to' is one of two things: Route-to-EL2+interruted-EL1, or Route-to-EL1+interrupted-EL2. Route-to-EL2+interrupted-EL1 is fine, regardless of SPSR_EL3.A the emulated SError can be delivered to EL2, as EL2 can't mask SError when executing at a lower EL. Route-to-EL1+interrupted-EL2 is the problem. SError is implicitly masked by running at a higher EL. Regardless of SPSR_EL3.A, the emulated SError can not be delivered. KVM does this on the way out of a guest, if an SError occurs during this time the CPU will wait until execution returns to EL1 before delivering the SError. Your firmware has to do the same. Table D1-15 in "D1.14.2 Asynchronous exception masking" has a table with all the combinations. The ARM-ARM is what we need to match with this behaviour. > but if it find that this SError come from EL0, it also need to deliver an > SError, right? I thought interrupted-EL0 could always be delivered: but re-reading the ARM-ARM's "D1.14.2 Asynchronous exception masking", if asynchronous exceptions are routed to EL1 then EL0&EL1 are treated the same. So if SError is routed to EL1, the exception interrupted EL0, and SPSR_EL3.A was set, you still can't deliver the emulated-SError you have to do the BERT-trick. Linux doesn't do this today, but another OS might (e.g. UEFI), and we might do this in the future. This is really tricky for firmware to get right. Another alternative would be to put the CPER records in a Polled buffer, unless something needs doing right now, in which case a BERT-reboot is probably best. Thanks, James ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v9 3/7] acpi: apei: Add SEI notification type support for ARMv8 2018-04-12 16:14 ` James Morse @ 2018-04-13 13:50 ` gengdongjiu 0 siblings, 0 replies; 10+ messages in thread From: gengdongjiu @ 2018-04-13 13:50 UTC (permalink / raw) To: James Morse, gengdongjiu Cc: lishuo1, merry.libing, linux-arm-kernel, Liujun (Jun Liu), linux-kernel, corbet, marc.zyngier, catalin.marinas, linux-doc, rjw, linux, will.deacon, robert.moore, linux-acpi, bp, lv.zheng, Huangshaoyu, kvmarm, devel James, Thanks for this mail. On 2018/4/13 0:14, James Morse wrote: > Hi gengdongjiu, > > On 12/04/18 06:00, gengdongjiu wrote: >> 2018-02-16 1:55 GMT+08:00 James Morse <james.morse@arm.com>: >>> On 05/02/18 11:24, gengdongjiu wrote: >>>>> Is the emulated SError routed following the routing rules for HCR_EL2.{AMO, >>>>> TGE}? >>>> >>>> Yes, it is. >>> >>> ... and yet ... >>> >>> >>>>> What does your firmware do when it wants to emulate SError but its masked? >>>>> (e.g.1: The physical-SError interrupted EL2 and the SPSR shows EL2 had >>>>> PSTATE.A set. >>>>> e.g.2: The physical-SError interrupted EL2 but HCR_EL2 indicates the >>>>> emulated SError should go to EL1. This effectively masks SError.) >>>> >>>> Currently we does not consider much about the mask status(SPSR). >>> >>> .. this is a problem. >>> >>> If you ignore SPSR_EL3 you may deliver an SError to EL1 when the exception >>> interrupted EL2. Even if you setup the EL1 register correctly, EL1 can't eret to >>> EL2. This should never happen, SError is effectively masked if you are running >>> at an EL higher than the one its routed to. >>> >>> More obviously: if the exception came from the EL that SError should be routed >>> to, but PSTATE.A was set, you can't deliver SError. Masking SError is the only > >> James, I summarized the masking and routing rules for SError to >> confirm with you for the firmware first solution, > > You also said "Currently we does not consider much about the mask status(SPSR)." Yes, we currently do not consider much it. After clarification with you, we want to modify the EL3 firmware to follow this rule. > > >> 1. If the HCR_EL2.{AMO,TGE} is set, > > If one or the other of these bits is set: (AMO==1 || TGE==1) > >> which means the SError should route to EL2, >> When system happens SError and trap to EL3, If EL3 find >> HCR_EL2.{AMO,TGE} and SPSR_EL3.A are both set, >> and find this SError come from EL2, it will not deliver an SError: >> store the RAS error in the BERT and 'reboot'; but if >> it find that this SError come from EL1 or EL0, it also need to deliver >> an SError, right? > > Yes. > > >> 2. If the HCR_EL2.{AMO,TGE} is not set, > > If neither of these bits is set: (AMO==0 && TGE == 0) > >> which means the SError should route to EL1, >> When system happens SError and trap to EL3, If EL3 find >> HCR_EL2.{AMO,TGE} and SPSR_EL3.A are both not set, > > (I'm reading this as all three of these bits are clear) sorry, it is a typo issue. it should be HCR_EL2.AMO and HCR_EL2.TGE are both clear, but SPSR_EL3.A is set. > >> and find this SError come from EL1, it will not deliver an SError: >> store the RAS error in the BERT and 'reboot'; > > No, (AMO==0 && TGE == 0) means SError is routed to EL1, this exception > interrupted EL1 and the A bit was clear, so EL1 can take an SError. Agree. > > The two cases here are: > AMO==0,TGE==0 means SError should be routed to EL1. If SPSR_EL3 says the > exception interrupted EL1 and the A bit was set, you need to do the BERT trick. > > If SPSR_EL3 says the exception interrupted EL2, you need to do the BERT trick "BERT trick" is storing the RAS error in the BERT and 'reboot, right? > regardless of the A bit, as SError is implicitly masked by running at a higher > exception level than it was routed to. > > >>From your v11 reply: >> 2. The exception came from the EL that SError should not be routed >> to(according to hcr_EL2.{AMO, TGE}),even though the PSTATE.A was set,EL3 >> firmware still deliver SError > > (this is re-iterating the two-cases above:) > 'not be routed to' is one of two things: Route-to-EL2+interruted-EL1, or > Route-to-EL1+interrupted-EL2. > > Route-to-EL2+interrupted-EL1 is fine, regardless of SPSR_EL3.A the emulated > SError can be delivered to EL2, as EL2 can't mask SError when executing at a > lower EL. Agree. > > Route-to-EL1+interrupted-EL2 is the problem. SError is implicitly masked by > running at a higher EL. Regardless of SPSR_EL3.A, the emulated SError can not be > delivered. "can not be delivered" means storing the RAS error in the BERT and 'reboot, right? In the Table D1-15 in "D1.14.2 Asynchronous exception masking", for the case, it is "C" "C"means SError is not taken regardless of the value of the Process state interrupt mask. for this case, whether it will be unsafe if BIOS directly reboot? > KVM does this on the way out of a guest, if an SError occurs during this time > the CPU will wait until execution returns to EL1 before delivering the SError. > Your firmware has to do the same. > > Table D1-15 in "D1.14.2 Asynchronous exception masking" has a table with all the > combinations. The ARM-ARM is what we need to match with this behaviour. > > >> but if it find that this SError come from EL0, it also need to deliver an >> SError, right? > > I thought interrupted-EL0 could always be delivered: but re-reading the > ARM-ARM's "D1.14.2 Asynchronous exception masking", if asynchronous exceptions > are routed to EL1 then EL0&EL1 are treated the same. > So if SError is routed to EL1, the exception interrupted EL0, and SPSR_EL3.A was > set, you still can't deliver the emulated-SError you have to do the BERT-trick. > Linux doesn't do this today, but another OS might (e.g. UEFI), and we might do > this in the future. For this case, whether it will be unsafe if BIOS directly reboot? For example, for some test purpose, EL0 set PSTATE.A, just right happen SError, then BIOS will reboot system. I am afraid that system will become unsafe because BIOS will reboot system. > > This is really tricky for firmware to get right. Another alternative would be to > put the CPER records in a Polled buffer, unless something needs doing right now, > in which case a BERT-reboot is probably best. In summary: [1]: Route-to-EL1 + interrupted-EL1, if SPSR_EL3.A is set, EL3 firmware can't deliver the emulated-SError, store the RAS error in the BERT and 'reboot. Route-to-EL2 + interrupted-EL2, if SPSR_EL3.A is set, EL3 firmware can't deliver the emulated-SError, store the RAS error in the BERT and 'reboot. I agree above two cases, but maybe we need to ensure that only in EL2 SError handler and EL1 SError exception handler the PSTATE.A is set, for other places, the PSTATE.A is not set. then BIOS can know this is nested-SError when find the SPSR_EL3.A is set, can we ensure that in the Linux kernel code and KVM code? [2]: Route-to-EL2 + interrupted-EL1, regardless of SPSR_EL3.A the emulated SError can be delivered to EL2. Route-to-EL2 + interrupted-EL0, regardless of SPSR_EL3.A the emulated SError can be delivered to EL2. I agree above two cases. [3]: Route-to-EL1+interrupted-EL0, if SPSR_EL3.A is set, EL3 firmware can't deliver the emulated-SError, store the RAS error in the BERT and 'reboot Route-to-EL1+interrupted-EL2, EL3 firmware store the RAS error in the BERT and 'reboot regardless of SPSR_EL3.A. For above two cases, I am worried system will become unsafe because BIOS will reboot system. > > > Thanks, > > James > > . > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v9 0/7] Handle guest RAS Error in KVM and kernel @ 2018-01-06 16:02 Dongjiu Geng 2018-01-06 16:02 ` [PATCH v9 3/7] acpi: apei: Add SEI notification type support for ARMv8 Dongjiu Geng 0 siblings, 1 reply; 10+ messages in thread From: Dongjiu Geng @ 2018-01-06 16:02 UTC (permalink / raw) To: christoffer.dall, marc.zyngier, linux, catalin.marinas, rjw, bp, robert.moore, lv.zheng, james.morse, corbet, will.deacon, linux-doc, linux-kernel, linux-arm-kernel, kvmarm, linux-acpi, devel Cc: gengdongjiu, huangshaoyu This series patches mainly do below things: 1. Trap guest RAS ERR* registers accesses to EL2 from Non-secure EL1, KVM will will do a minimum simulation, these registers are simulated to RAZ/WI in KVM. 2. Route guest synchronous External Abort to EL2. If it is also routed to EL3 by firmware at the same time, system will trap to EL3 firmware instead of EL2 KVM, then firmware judges whether EL2 routing is enabled, if enabled, jump back to EL2 KVM, otherwise jump back to EL1 host kernel. 3. Enable APEI ARv8 SEI notification to parse the CPER records for SError in the ACPI GHES driver, KVM will call handle_guest_sei() to let ACPI driver to parse the CPER recorded for SError which happened in the guest 4. If ACPI driver parsed the CPER record failed, KVM will classify the Error through Exception Syndrome Register and do different approaches according to Asynchronous Error Type 5. If the guest RAS SError is not propagated and not consumed, this exception is precise, we temporarily shut down the VM to isolate the error. For other Asynchronous Error Type, KVM directly injects virtual SError with IMPLEMENTATION DEFINED ESR or KVM panic if the error is fatal. For the RAS extension, guest virtual ESR must be set, because all-zero means 'RAS error: Uncategorized' instead of 'no valid ISS', so set this ESR to IMPLEMENTATION DEFINED by default if user space does not specify it. change since v8: 1. update the patch [1/7] and [2/7] to align this serie. https://www.spinics.net/lists/arm-kernel/msg623513.html https://www.spinics.net/lists/arm-kernel/msg623520.html 2. In kvm ,check handle_guest_sei()'s return value. If this function return true, stop classifying errors. 3. Temporarily shut down the VM to isolate the error for recoverable error (UER) 4. update some patch's commit messages and clean some patches Dongjiu Geng (5): acpi: apei: Add SEI notification type support for ARMv8 KVM: arm64: Trap RAS error registers and set HCR_EL2's TERR & TEA arm64: kvm: Introduce KVM_ARM_SET_SERROR_ESR ioctl arm64: kvm: Set Virtual SError Exception Syndrome for guest arm64: kvm: handle guest SError Interrupt by categorization James Morse (1): KVM: arm64: Save ESR_EL2 on guest SError Xie XiuQi (1): arm64: cpufeature: Detect CPU RAS Extentions Documentation/virtual/kvm/api.txt | 11 ++++++ arch/arm/include/asm/kvm_host.h | 1 + arch/arm/kvm/guest.c | 9 +++++ arch/arm64/Kconfig | 16 +++++++++ arch/arm64/include/asm/cpucaps.h | 3 +- arch/arm64/include/asm/esr.h | 11 ++++++ arch/arm64/include/asm/kvm_arm.h | 2 ++ arch/arm64/include/asm/kvm_emulate.h | 17 +++++++++ arch/arm64/include/asm/kvm_host.h | 2 ++ arch/arm64/include/asm/sysreg.h | 15 ++++++++ arch/arm64/include/asm/system_misc.h | 1 + arch/arm64/kernel/cpufeature.c | 13 +++++++ arch/arm64/kvm/guest.c | 14 ++++++++ arch/arm64/kvm/handle_exit.c | 68 +++++++++++++++++++++++++++++++++--- arch/arm64/kvm/hyp/switch.c | 25 +++++++++++-- arch/arm64/kvm/inject_fault.c | 13 ++++++- arch/arm64/kvm/reset.c | 3 ++ arch/arm64/kvm/sys_regs.c | 10 ++++++ arch/arm64/mm/fault.c | 16 +++++++++ drivers/acpi/apei/Kconfig | 15 ++++++++ drivers/acpi/apei/ghes.c | 53 ++++++++++++++++++++++++++++ include/acpi/ghes.h | 1 + include/uapi/linux/kvm.h | 3 ++ virt/kvm/arm/arm.c | 7 ++++ 24 files changed, 320 insertions(+), 9 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v9 3/7] acpi: apei: Add SEI notification type support for ARMv8 2018-01-06 16:02 [PATCH v9 0/7] Handle guest RAS Error in KVM and kernel Dongjiu Geng @ 2018-01-06 16:02 ` Dongjiu Geng 2018-01-22 19:39 ` James Morse 0 siblings, 1 reply; 10+ messages in thread From: Dongjiu Geng @ 2018-01-06 16:02 UTC (permalink / raw) To: christoffer.dall, marc.zyngier, linux, catalin.marinas, rjw, bp, robert.moore, lv.zheng, james.morse, corbet, will.deacon, linux-doc, linux-kernel, linux-arm-kernel, kvmarm, linux-acpi, devel Cc: gengdongjiu, huangshaoyu ARMv8.2 requires implementation of the RAS extension. In this extension, it adds SEI(SError Interrupt) notification type, this patch adds new GHES error source SEI handling functions. This error source parsing and handling method is similar with the SEA. Expose API ghes_notify_sei() to external users. External modules can call this exposed API to parse APEI table and handle the SEI notification. Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com> --- drivers/acpi/apei/Kconfig | 15 ++++++++++++++ drivers/acpi/apei/ghes.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++ include/acpi/ghes.h | 1 + 3 files changed, 69 insertions(+) diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig index 52ae543..ff4afc3 100644 --- a/drivers/acpi/apei/Kconfig +++ b/drivers/acpi/apei/Kconfig @@ -55,6 +55,21 @@ config ACPI_APEI_SEA option allows the OS to look for such hardware error record, and take appropriate action. +config ACPI_APEI_SEI + bool "APEI SError(System Error) Interrupt logging/recovering support" + depends on ARM64 && ACPI_APEI_GHES + default y + help + This option should be enabled if the system supports + firmware first handling of SEI (SError interrupt). + + SEI happens with asynchronous external abort for errors on device + memory reads on ARMv8 systems. If a system supports firmware first + handling of SEI, the platform analyzes and handles hardware error + notifications from SEI, and it may then form a hardware error record for + the OS to parse and handle. This option allows the OS to look for + such hardware error record, and take appropriate action. + config ACPI_APEI_MEMORY_FAILURE bool "APEI memory error recovering support" depends on ACPI_APEI && MEMORY_FAILURE diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index 6a3f824..67cd3a7 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -855,6 +855,46 @@ static inline void ghes_sea_add(struct ghes *ghes) { } static inline void ghes_sea_remove(struct ghes *ghes) { } #endif /* CONFIG_ACPI_APEI_SEA */ +#ifdef CONFIG_ACPI_APEI_SEI +static LIST_HEAD(ghes_sei); + +/* + * Return 0 only if one of the SEI error sources successfully reported an error + * record sent from the firmware. + */ +int ghes_notify_sei(void) +{ + struct ghes *ghes; + int ret = -ENOENT; + + rcu_read_lock(); + list_for_each_entry_rcu(ghes, &ghes_sei, list) { + if (!ghes_proc(ghes)) + ret = 0; + } + rcu_read_unlock(); + return ret; +} + +static void ghes_sei_add(struct ghes *ghes) +{ + mutex_lock(&ghes_list_mutex); + list_add_rcu(&ghes->list, &ghes_sei); + mutex_unlock(&ghes_list_mutex); +} + +static void ghes_sei_remove(struct ghes *ghes) +{ + mutex_lock(&ghes_list_mutex); + list_del_rcu(&ghes->list); + mutex_unlock(&ghes_list_mutex); + synchronize_rcu(); +} +#else /* CONFIG_ACPI_APEI_SEI */ +static inline void ghes_sei_add(struct ghes *ghes) { } +static inline void ghes_sei_remove(struct ghes *ghes) { } +#endif /* CONFIG_ACPI_APEI_SEI */ + #ifdef CONFIG_HAVE_ACPI_APEI_NMI /* * printk is not safe in NMI context. So in NMI handler, we allocate @@ -1086,6 +1126,13 @@ static int ghes_probe(struct platform_device *ghes_dev) goto err; } break; + case ACPI_HEST_NOTIFY_SEI: + if (!IS_ENABLED(CONFIG_ACPI_APEI_SEI)) { + pr_warn(GHES_PFX "Generic hardware error source: %d notified via SEI is not supported!\n", + generic->header.source_id); + goto err; + } + break; case ACPI_HEST_NOTIFY_NMI: if (!IS_ENABLED(CONFIG_HAVE_ACPI_APEI_NMI)) { pr_warn(GHES_PFX "Generic hardware error source: %d notified via NMI interrupt is not supported!\n", @@ -1158,6 +1205,9 @@ static int ghes_probe(struct platform_device *ghes_dev) case ACPI_HEST_NOTIFY_SEA: ghes_sea_add(ghes); break; + case ACPI_HEST_NOTIFY_SEI: + ghes_sei_add(ghes); + break; case ACPI_HEST_NOTIFY_NMI: ghes_nmi_add(ghes); break; @@ -1211,6 +1261,9 @@ static int ghes_remove(struct platform_device *ghes_dev) case ACPI_HEST_NOTIFY_SEA: ghes_sea_remove(ghes); break; + case ACPI_HEST_NOTIFY_SEI: + ghes_sei_remove(ghes); + break; case ACPI_HEST_NOTIFY_NMI: ghes_nmi_remove(ghes); break; diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h index 8feb0c8..9ba59e2 100644 --- a/include/acpi/ghes.h +++ b/include/acpi/ghes.h @@ -120,5 +120,6 @@ static inline void *acpi_hest_get_next(struct acpi_hest_generic_data *gdata) section = acpi_hest_get_next(section)) int ghes_notify_sea(void); +int ghes_notify_sei(void); #endif /* GHES_H */ -- 1.9.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v9 3/7] acpi: apei: Add SEI notification type support for ARMv8 2018-01-06 16:02 ` [PATCH v9 3/7] acpi: apei: Add SEI notification type support for ARMv8 Dongjiu Geng @ 2018-01-22 19:39 ` James Morse 2018-01-23 9:23 ` gengdongjiu 0 siblings, 1 reply; 10+ messages in thread From: James Morse @ 2018-01-22 19:39 UTC (permalink / raw) To: Dongjiu Geng Cc: christoffer.dall, marc.zyngier, linux, catalin.marinas, rjw, bp, robert.moore, lv.zheng, corbet, will.deacon, linux-doc, linux-kernel, linux-arm-kernel, kvmarm, linux-acpi, devel, huangshaoyu Hi Dongjiu Geng, (versions of patches 1,2 and 4 have been queued by Catalin) (Nit 'ACPI / APEI:' is the normal subject prefix for ghes.c, this helps the maintainers know which patches they need to pay attention to when you are touching multiple trees) On 06/01/18 16:02, Dongjiu Geng wrote: > ARMv8.2 requires implementation of the RAS extension. > In > this extension, it adds SEI(SError Interrupt) notification > type, this patch adds new GHES error source SEI handling > functions. This reads as if this patch is handling SError RAS notifications generated by a CPU with the RAS extensions. These are about CPU->Software notifications. APEI and GHES are a firmware first mechanism which is Software->Software. Reading the v8.2 documents won't help anyone with the APEI/GHES code. Please describe this from the ACPI view, "ACPI 6.x adds support for NOTIFY_SEI as a GHES notification mechanism... ", its up to the arch code to spot a v8.2 RAS Error based on the cpu caps. > This error source parsing and handling method > is similar with the SEA. There are problems with doing this: Oct. 18, 2017, 10:26 a.m. James Morse wrote: | How do SEA and SEI interact? | | As far as I can see they can both interrupt each other, which isn't something | the single in_nmi() path in APEI can handle. I thinks we should fix this | first. [..] | SEA gets away with a lot of things because its synchronous. SEI isn't. Xie | XiuQi pointed to the memory_failure_queue() code. We can use this directly | from SEA, but not SEI. (what happens if an SError arrives while we are | queueing memory_failure work from an IRQ). | | The one that scares me is the trace-point reporting stuff. What happens if an | SError arrives while we are enabling a trace point? (these are static-keys | right?) | | I don't think we can just plumb SEI in like this and be done with it. | (I'm looking at teasing out the estatus cache code from being x86:NMI only. | This way we solve the same 'cant do this from NMI context' with the same | code'.) I will post what I've got for this estatus-cache thing as an RFC, its not ready to be considered yet. > Expose API ghes_notify_sei() to external users. External > modules can call this exposed API to parse APEI table and > handle the SEI notification. external modules? You mean called by the arch code when it gets this NOTIFY_SEI? Thanks, James ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v9 3/7] acpi: apei: Add SEI notification type support for ARMv8 2018-01-22 19:39 ` James Morse @ 2018-01-23 9:23 ` gengdongjiu 2018-01-23 10:07 ` gengdongjiu 2018-01-30 19:39 ` James Morse 0 siblings, 2 replies; 10+ messages in thread From: gengdongjiu @ 2018-01-23 9:23 UTC (permalink / raw) To: James Morse Cc: christoffer.dall, marc.zyngier, linux, catalin.marinas, rjw, bp, robert.moore, lv.zheng, corbet, will.deacon, linux-doc, linux-kernel, linux-arm-kernel, kvmarm, linux-acpi, devel, huangshaoyu Hi James, On 2018/1/23 3:39, James Morse wrote: > Hi Dongjiu Geng, > > (versions of patches 1,2 and 4 have been queued by Catalin) > > (Nit 'ACPI / APEI:' is the normal subject prefix for ghes.c, this helps the > maintainers know which patches they need to pay attention to when you are > touching multiple trees) > > On 06/01/18 16:02, Dongjiu Geng wrote: >> ARMv8.2 requires implementation of the RAS extension. > >> In >> this extension, it adds SEI(SError Interrupt) notification >> type, this patch adds new GHES error source SEI handling >> functions. > > This reads as if this patch is handling SError RAS notifications generated by a > CPU with the RAS extensions. These are about CPU->Software notifications. APEI > and GHES are a firmware first mechanism which is Software->Software. > Reading the v8.2 documents won't help anyone with the APEI/GHES code. > > Please describe this from the ACPI view, "ACPI 6.x adds support for NOTIFY_SEI > as a GHES notification mechanism... ", its up to the arch code to spot a v8.2 > RAS Error based on the cpu caps.Ok, I will modify it. > > >> This error source parsing and handling method >> is similar with the SEA. > > There are problems with doing this: > > Oct. 18, 2017, 10:26 a.m. James Morse wrote: > | How do SEA and SEI interact? > | > | As far as I can see they can both interrupt each other, which isn't something > | the single in_nmi() path in APEI can handle. I thinks we should fix this > | first. > > [..] > > | SEA gets away with a lot of things because its synchronous. SEI isn't. Xie > | XiuQi pointed to the memory_failure_queue() code. We can use this directly > | from SEA, but not SEI. (what happens if an SError arrives while we are > | queueing memory_failure work from an IRQ). > | > | The one that scares me is the trace-point reporting stuff. What happens if an > | SError arrives while we are enabling a trace point? (these are static-keys > | right?) > | > | I don't think we can just plumb SEI in like this and be done with it. > | (I'm looking at teasing out the estatus cache code from being x86:NMI only. > | This way we solve the same 'cant do this from NMI context' with the same > | code'.) > > > I will post what I've got for this estatus-cache thing as an RFC, its not ready > to be considered yet.Yes, I know you are dong that. Your serial's patch will consider all above things, right? If your patch can be consider that, this patch can based on your patchset. thanks. > > >> Expose API ghes_notify_sei() to external users. External >> modules can call this exposed API to parse APEI table and >> handle the SEI notification. > > external modules? You mean called by the arch code when it gets this NOTIFY_SEI? yes, called by kernel ARCH code, such as below, I remember I have discussed with you. asmlinkage void do_serror(struct pt_regs *regs, unsigned int esr) { nmi_enter(); if (!ghes_notify_sei()) return; /* non-RAS errors are not containable */ if (!arm64_is_ras_serror(esr) || arm64_is_fatal_ras_serror(regs, esr)) arm64_serror_panic(regs, esr); nmi_exit(); } > > > Thanks, > > James > > . > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v9 3/7] acpi: apei: Add SEI notification type support for ARMv8 2018-01-23 9:23 ` gengdongjiu @ 2018-01-23 10:07 ` gengdongjiu 2018-01-30 19:39 ` James Morse 1 sibling, 0 replies; 10+ messages in thread From: gengdongjiu @ 2018-01-23 10:07 UTC (permalink / raw) To: James Morse Cc: christoffer.dall, marc.zyngier, linux, catalin.marinas, rjw, bp, robert.moore, lv.zheng, corbet, will.deacon, linux-doc, linux-kernel, linux-arm-kernel, kvmarm, linux-acpi, devel, huangshaoyu sorry fix a typo. On 2018/1/23 17:23, gengdongjiu wrote: >> There are problems with doing this: >> >> Oct. 18, 2017, 10:26 a.m. James Morse wrote: >> | How do SEA and SEI interact? >> | >> | As far as I can see they can both interrupt each other, which isn't something >> | the single in_nmi() path in APEI can handle. I thinks we should fix this >> | first. >> >> [..] >> >> | SEA gets away with a lot of things because its synchronous. SEI isn't. Xie >> | XiuQi pointed to the memory_failure_queue() code. We can use this directly >> | from SEA, but not SEI. (what happens if an SError arrives while we are >> | queueing memory_failure work from an IRQ). >> | >> | The one that scares me is the trace-point reporting stuff. What happens if an >> | SError arrives while we are enabling a trace point? (these are static-keys >> | right?) >> | >> | I don't think we can just plumb SEI in like this and be done with it. >> | (I'm looking at teasing out the estatus cache code from being x86:NMI only. >> | This way we solve the same 'cant do this from NMI context' with the same >> | code'.) >> >> >> I will post what I've got for this estatus-cache thing as an RFC, its not ready >> to be considered yet. Yes, I know you are dong that. Your serial's patch will consider all above things, right? If your patch can be consider that, this patch can based on your patchset. thanks. > >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v9 3/7] acpi: apei: Add SEI notification type support for ARMv8 2018-01-23 9:23 ` gengdongjiu 2018-01-23 10:07 ` gengdongjiu @ 2018-01-30 19:39 ` James Morse 1 sibling, 0 replies; 10+ messages in thread From: James Morse @ 2018-01-30 19:39 UTC (permalink / raw) To: gengdongjiu Cc: christoffer.dall, marc.zyngier, linux, catalin.marinas, rjw, bp, robert.moore, lv.zheng, corbet, will.deacon, linux-doc, linux-kernel, linux-arm-kernel, kvmarm, linux-acpi, devel, huangshaoyu Hi gengdongjiu, On 23/01/18 09:23, gengdongjiu wrote: > On 2018/1/23 3:39, James Morse wrote: >> gengdongjiu wrote: >>> This error source parsing and handling method >>> is similar with the SEA. >> >> There are problems with doing this: >> >> Oct. 18, 2017, 10:26 a.m. James Morse wrote: >> | How do SEA and SEI interact? >> | >> | As far as I can see they can both interrupt each other, which isn't something >> | the single in_nmi() path in APEI can handle. I thinks we should fix this >> | first. >> >> [..] >> >> | SEA gets away with a lot of things because its synchronous. SEI isn't. Xie >> | XiuQi pointed to the memory_failure_queue() code. We can use this directly >> | from SEA, but not SEI. (what happens if an SError arrives while we are >> | queueing memory_failure work from an IRQ). >> | >> | The one that scares me is the trace-point reporting stuff. What happens if an >> | SError arrives while we are enabling a trace point? (these are static-keys >> | right?) >> | >> | I don't think we can just plumb SEI in like this and be done with it. >> | (I'm looking at teasing out the estatus cache code from being x86:NMI only. >> | This way we solve the same 'cant do this from NMI context' with the same >> | code'.) >> >> >> I will post what I've got for this estatus-cache thing as an RFC, its not ready >> to be considered yet. > Yes, I know you are dong that. Your serial's patch will consider all above things, right? Assuming I got it right, yes. It currently makes the race Xie XiuQi spotted worse, which I want to fix too. (details on the cover letter) > If your patch can be consider that, this patch can based on your patchset. thanks. I'd like to pick these patches onto the end of that series, but first I want to know what NOTIFY_SEI means for any OS. The ACPI spec doesn't say, and because its asynchronous, route-able and mask-able, there are many more corners than NOTFIY_SEA. This thing is a notification using an emulated SError exception. (emulated because physical-SError must be routed to EL3 for firmware-first, and virtual-SError belongs to EL2). Does your firmware emulate SError exactly as the TakeException() pseudo code in the Arm-Arm? Is the emulated SError routed following the routing rules for HCR_EL2.{AMO, TGE}? What does your firmware do when it wants to emulate SError but its masked? (e.g.1: The physical-SError interrupted EL2 and the SPSR shows EL2 had PSTATE.A set. e.g.2: The physical-SError interrupted EL2 but HCR_EL2 indicates the emulated SError should go to EL1. This effectively masks SError.) Answers to these let us determine whether a bug is in the firmware or the kernel. If firmware is expecting the OS to do something special, I'd like to know about it from the beginning! >>> Expose API ghes_notify_sei() to external users. External >>> modules can call this exposed API to parse APEI table and >>> handle the SEI notification. >> >> external modules? You mean called by the arch code when it gets this NOTIFY_SEI? > yes, called by kernel ARCH code, such as below, I remember I have discussed with you. Sure. The phrase 'external modules' usually means the '.ko' files that live in /lib/modules, nothing outside the kernel tree should be doing this stuff. Thanks, James ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-04-13 13:52 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-05 11:24 [PATCH v9 3/7] acpi: apei: Add SEI notification type support for ARMv8 gengdongjiu 2018-02-15 17:55 ` James Morse 2018-04-12 5:00 ` gengdongjiu 2018-04-12 16:14 ` James Morse 2018-04-13 13:50 ` gengdongjiu -- strict thread matches above, loose matches on Subject: below -- 2018-01-06 16:02 [PATCH v9 0/7] Handle guest RAS Error in KVM and kernel Dongjiu Geng 2018-01-06 16:02 ` [PATCH v9 3/7] acpi: apei: Add SEI notification type support for ARMv8 Dongjiu Geng 2018-01-22 19:39 ` James Morse 2018-01-23 9:23 ` gengdongjiu 2018-01-23 10:07 ` gengdongjiu 2018-01-30 19:39 ` James Morse
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).