diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.cindex 105b6ab..a96594f 100644--- a/arch/arm/kvm/mmu.c+++ b/arch/arm/kvm/mmu.c
+static void kvm_handle_bad_page(unsigned long address,
+ bool hugetlb, bool hwpoison)
+{
+ /* handle both hwpoison and other synchronous external Abort */
+ if (hwpoison)
+ kvm_send_signal(address, hugetlb, true);
+ else
+ kvm_send_signal(address, hugetlb, false);
+}
Why the extra level of indirection? We only want to signal userspace like this
from KVM for hwpoison. Signals for RAS related reasons should come from the bits
of the kernel that decoded the error.
For the SEA, the are maily two types:
0b010000 Synchronous External Abort on memory access.
0b0101xx Synchronous External Abort on page table walk. DFSC[1:0]
encode the level.
(KVM shouldn't have to make decisions about this)
hwpoison should belong to the "Synchronous External Abort on memory access"
if the SEA type is not hwpoison, such as page table walk, do you mean
KVM do not deliver the SIGBUS?
The flow of events should be SEI/SEA from firmware to the hosts's APEI code. KVM
should only be involved to get us back to the host if we were running a guest.
The APEI/hwpoison code may cause a set of processes to be sent signals. The code
in mm/memory-failure.c does this by walking the process rmaps using the physical
addresses in the CPER records.
We want user space to be sent signals as this can (and should) work in exactly
the same way on arm64 as it does on x86 or any other architecture. If a
web-browser can handle SIGBUS notifications for memory-corruption, it shouldn't
have to care what architecture it is running on.
So what is that KVM+SIGBUS patch about?...
quoted
(hwpoison for KVM is a corner case as Qemu's memory effectively has two users,
Qemu and KVM. This isn't the example of how user-space gets signalled.)
KVM creates guests as if they were additional users of Qemu's memory. The code
in mm/memory-failure.c may find that Qemu didn't have the affected page mapped
to user-space - but it may have been in use by stage2.
The KVM+SIGBUS patch hides this difference, meaning Qemu gets a signal when the
guest touches the hwpoison page as if Qemu had touched the page itself.
Signals from KVM is a corner case, for firmware-first decisions should happen in
the APEI code based on CPER records.
If so, how the KVM handle the SEA type other than hwpoison?
To deliver to a guest? It shouldn't have to know, user space should use a KVM
API to drive this.
When received from hardware? It shouldn't have to care, these things should be
passed into the APEI code for handling. KVM just needs to put the host registers
back.
Why do we need a userspace API for SEA? It can also be done by using
KVM_{G,S}ET_ONE_REG to change the vcpu registers. The advantage of doing it this
way is you can choose which ESR value to use.
Adding a new API call to do something you could do with an old one doesn't look
right.
James, I considered your suggestion before that use the
KVM_{G,S}ET_ONE_REG to change the vcpu registers. but I found it does
not have difference to use the alread existed KVM API.
(Only that is an in-kernel helper, not a published API)
so may be
changing the vcpu registers in qemu will duplicate with the KVM APIs.
That is true, but the alternative is a new API that doesn't do anything new, its
just more convenient.
Marc and Christoffer are the people to convince.
I argue the existing API is sufficient.
injection a SEA is no more than setting some registers: elr_el1, PC,
PSTATE, SPSR_el1, far_el1, esr_el1
I seen this KVM API do the same thing as Qemu. do you found call this
API will have issue and necessary to choose another ESR value?
Should we let user-space pick the ESR to deliver to the guest? Yes, letting
user-space specify the ESR gives the most flexibility to do something clever in
the future. An obvious choice for SEA is between the external-abort and 'parity
or ECC error' codes. If we tell user-space which of these happened (I don't
think Linux does today) then Qemu can relay that information to the guest.
Thanks,
James
Why do we need a userspace API for SEA? It can also be done by using
KVM_{G,S}ET_ONE_REG to change the vcpu registers. The advantage of doing it this
way is you can choose which ESR value to use.
Adding a new API call to do something you could do with an old one doesn't look
right.
James, I considered your suggestion before that use the
KVM_{G,S}ET_ONE_REG to change the vcpu registers. but I found it does
not have difference to use the alread existed KVM API.
(Only that is an in-kernel helper, not a published API)
quoted
so may be
changing the vcpu registers in qemu will duplicate with the KVM APIs.
That is true, but the alternative is a new API that doesn't do anything new, its
just more convenient.
Marc and Christoffer are the people to convince.
I argue the existing API is sufficient.
I must admit I am losing track of exactly what this proposed API was
supposed to do.
However, if it's a question about setting up VCPU registers to a certain
state and potentially modifying memory, then I think experience has
shown us (psci) that emulating something in the kernel that userspace
can have fine-grained control over is a bad idea, and should be left to
userspace using as generic APIs as possible.
Furthermore, if I understand what injecting a SEA requires, it is very
similar to resetting the CPU and loading data into guest memory, which
QEMU already does today, and there is no reason to introduce additional
APIs if it can be done using KVM_GET/SET_ONE_REG ioctls.
Thanks,
-Christoffer
From: James Morse <james.morse@arm.com> Date: 2017-05-09 14:29:02
Hi Christoffer,
On 08/05/17 18:54, Christoffer Dall wrote:
On Mon, May 08, 2017 at 06:28:02PM +0100, James Morse wrote:
I must admit I am losing track of exactly what this proposed API was
supposed to do.
There are two, and we keep jumping between them!
This is about two notification methods APEI has for arm64, 'SEA' and 'SEI'.
SEA is synchronous and looks like a data abort. Qemu/kvmtool can inject these
today using the KVM_GET/SET_ONE_REG API whenever it wants to.
SEI uses SError, is asynchronous and can be masked. In addition these need to be
consumed/synchronised by the ESB instruction, even when executed by a guest.
Hardware has the necessary bits to drive all this, we need to expose an API to
drive it.
(I try to spell them out each time so I don't confuse SEI with something
synchronous!)
This patch was about SEA. I think you've answered our question:
However, if it's a question about setting up VCPU registers to a certain
state and potentially modifying memory, then I think experience has
shown us (psci) that emulating something in the kernel that userspace
can have fine-grained control over is a bad idea, and should be left to
userspace using as generic APIs as possible.
Furthermore, if I understand what injecting a SEA requires, it is very
similar to resetting the CPU and loading data into guest memory, which
QEMU already does today, and there is no reason to introduce additional
APIs if it can be done using KVM_GET/SET_ONE_REG ioctls.
diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.cindex 105b6ab..a96594f 100644--- a/arch/arm/kvm/mmu.c+++ b/arch/arm/kvm/mmu.c
+static void kvm_handle_bad_page(unsigned long address,
+ bool hugetlb, bool hwpoison)
+{
+ /* handle both hwpoison and other synchronous external Abort */
+ if (hwpoison)
+ kvm_send_signal(address, hugetlb, true);
+ else
+ kvm_send_signal(address, hugetlb, false);
+}
Why the extra level of indirection? We only want to signal userspace like this
from KVM for hwpoison. Signals for RAS related reasons should come from the bits
of the kernel that decoded the error.
For the SEA, the are maily two types:
0b010000 Synchronous External Abort on memory access.
0b0101xx Synchronous External Abort on page table walk. DFSC[1:0]
encode the level.
(KVM shouldn't have to make decisions about this)
quoted
hwpoison should belong to the "Synchronous External Abort on memory access"
if the SEA type is not hwpoison, such as page table walk, do you mean
KVM do not deliver the SIGBUS?
The flow of events should be SEI/SEA from firmware to the hosts's APEI code. KVM
should only be involved to get us back to the host if we were running a guest.
The APEI/hwpoison code may cause a set of processes to be sent signals. The code
in mm/memory-failure.c does this by walking the process rmaps using the physical
addresses in the CPER records.
We want user space to be sent signals as this can (and should) work in exactly
the same way on arm64 as it does on x86 or any other architecture. If a
web-browser can handle SIGBUS notifications for memory-corruption, it shouldn't
have to care what architecture it is running on.
Ok, James, understand.
So what is that KVM+SIGBUS patch about?...
quoted
quoted
(hwpoison for KVM is a corner case as Qemu's memory effectively has two users,
Qemu and KVM. This isn't the example of how user-space gets signalled.)
KVM creates guests as if they were additional users of Qemu's memory. The code
in mm/memory-failure.c may find that Qemu didn't have the affected page mapped
to user-space - but it may have been in use by stage2.
The KVM+SIGBUS patch hides this difference, meaning Qemu gets a signal when the
guest touches the hwpoison page as if Qemu had touched the page itself.
Signals from KVM is a corner case, for firmware-first decisions should happen in
the APEI code based on CPER records.
quoted
If so, how the KVM handle the SEA type other than hwpoison?
To deliver to a guest? It shouldn't have to know, user space should use a KVM
API to drive this.
When received from hardware? It shouldn't have to care, these things should be
passed into the APEI code for handling. KVM just needs to put the host registers
back.
Recently I confirmed with the hardware team. they said almost all the SEA errors have the
Poison flag, so may be there is no need to consider other SEA errors other than hwPoison.
only consider SEA hwpoison errors can be enough.
Why do we need a userspace API for SEA? It can also be done by using
KVM_{G,S}ET_ONE_REG to change the vcpu registers. The advantage of doing it this
way is you can choose which ESR value to use.
Adding a new API call to do something you could do with an old one doesn't look
right.
James, I considered your suggestion before that use the
KVM_{G,S}ET_ONE_REG to change the vcpu registers. but I found it does
not have difference to use the alread existed KVM API.
(Only that is an in-kernel helper, not a published API)
yes, the kvm_inject_dabt is an in-kernel API.
quoted
so may be
changing the vcpu registers in qemu will duplicate with the KVM APIs.
That is true, but the alternative is a new API that doesn't do anything new, its
just more convenient.
Marc and Christoffer are the people to convince.
I argue the existing API is sufficient.
quoted
injection a SEA is no more than setting some registers: elr_el1, PC,
PSTATE, SPSR_el1, far_el1, esr_el1
I seen this KVM API do the same thing as Qemu. do you found call this
API will have issue and necessary to choose another ESR value?
Should we let user-space pick the ESR to deliver to the guest? Yes, letting
user-space specify the ESR gives the most flexibility to do something clever in
the future. An obvious choice for SEA is between the external-abort and 'parity
or ECC error' codes. If we tell user-space which of these happened (I don't
think Linux does today) then Qemu can relay that information to the guest.
may be the ESR is delivered by the KVM.
(1) guest OS EL0 happen SEA due to hwpoison
(2) CPU traps to EL3 firmware, and update the ESR_EL3
(3) the EL3 firmware copies the ESR_EL3 to ESR_EL2
(4) then jump to EL2 hypervisor, hypervisor uses the ESR_EL2 to inject the SEA.
May be the esr_el2 can provide the accurate error information.
or do you think user-space specify the ESR instead of esr_el2 is better?
Thanks James's explanation.
Hi Christoffer,
On 2017/5/9 22:28, James Morse wrote:
Hi Christoffer,
On 08/05/17 18:54, Christoffer Dall wrote:
quoted
On Mon, May 08, 2017 at 06:28:02PM +0100, James Morse wrote:
I must admit I am losing track of exactly what this proposed API was
supposed to do.
There are two, and we keep jumping between them!
This is about two notification methods APEI has for arm64, 'SEA' and 'SEI'.
SEA is synchronous and looks like a data abort. Qemu/kvmtool can inject these
today using the KVM_GET/SET_ONE_REG API whenever it wants to.
SEI uses SError, is asynchronous and can be masked. In addition these need to be
consumed/synchronised by the ESB instruction, even when executed by a guest.
Hardware has the necessary bits to drive all this, we need to expose an API to
drive it.
(I try to spell them out each time so I don't confuse SEI with something
synchronous!)
This patch was about SEA. I think you've answered our question:
we are talking about the SEA(synchronous data abort) injection two methods:
(1)change vcpu registers in the Qemu/kvmtools and using the KVM_GET/SET_ONE_REG API to set.
(2)using existed in-kernel API "kvm_inject_dabt" to inject through IOCTL command from Qemu.
quoted
However, if it's a question about setting up VCPU registers to a certain
state and potentially modifying memory, then I think experience has
shown us (psci) that emulating something in the kernel that userspace
can have fine-grained control over is a bad idea, and should be left to
userspace using as generic APIs as possible.
Furthermore, if I understand what injecting a SEA requires, it is very
similar to resetting the CPU and loading data into guest memory, which
QEMU already does today, and there is no reason to introduce additional
APIs if it can be done using KVM_GET/SET_ONE_REG ioctls.
From: Christoffer Dall <hidden> Date: 2017-05-10 12:21:07
On Wed, May 10, 2017 at 05:15:04PM +0800, gengdongjiu wrote:
Thanks James's explanation.
Hi Christoffer,
On 2017/5/9 22:28, James Morse wrote:
quoted
Hi Christoffer,
On 08/05/17 18:54, Christoffer Dall wrote:
quoted
On Mon, May 08, 2017 at 06:28:02PM +0100, James Morse wrote:
I must admit I am losing track of exactly what this proposed API was
supposed to do.
There are two, and we keep jumping between them!
This is about two notification methods APEI has for arm64, 'SEA' and 'SEI'.
SEA is synchronous and looks like a data abort. Qemu/kvmtool can inject these
today using the KVM_GET/SET_ONE_REG API whenever it wants to.
SEI uses SError, is asynchronous and can be masked. In addition these need to be
consumed/synchronised by the ESB instruction, even when executed by a guest.
Hardware has the necessary bits to drive all this, we need to expose an API to
drive it.
(I try to spell them out each time so I don't confuse SEI with something
synchronous!)
This patch was about SEA. I think you've answered our question:
we are talking about the SEA(synchronous data abort) injection two methods:
(1)change vcpu registers in the Qemu/kvmtools and using the KVM_GET/SET_ONE_REG API to set.
Yes, if this is possible, why would you want something more?
(2)using existed in-kernel API "kvm_inject_dabt" to inject through IOCTL command from Qemu.
I'm not really going to consider this, because "use internal API from
userspace" doesn't work.
So this should be:
(2) Introduce a new API to do X.
I still think you know what my preference is; use the existing API if at
all possible.
Thanks,
-Christoffer
Hi Christoffer,
On 2017/5/10 20:20, Christoffer Dall wrote:
On Wed, May 10, 2017 at 05:15:04PM +0800, gengdongjiu wrote:
quoted
Thanks James's explanation.
Hi Christoffer,
On 2017/5/9 22:28, James Morse wrote:
quoted
Hi Christoffer,
On 08/05/17 18:54, Christoffer Dall wrote:
quoted
On Mon, May 08, 2017 at 06:28:02PM +0100, James Morse wrote:
I must admit I am losing track of exactly what this proposed API was
supposed to do.
There are two, and we keep jumping between them!
This is about two notification methods APEI has for arm64, 'SEA' and 'SEI'.
SEA is synchronous and looks like a data abort. Qemu/kvmtool can inject these
today using the KVM_GET/SET_ONE_REG API whenever it wants to.
SEI uses SError, is asynchronous and can be masked. In addition these need to be
consumed/synchronised by the ESB instruction, even when executed by a guest.
Hardware has the necessary bits to drive all this, we need to expose an API to
drive it.
(I try to spell them out each time so I don't confuse SEI with something
synchronous!)
This patch was about SEA. I think you've answered our question:
we are talking about the SEA(synchronous data abort) injection two methods:
(1)change vcpu registers in the Qemu/kvmtools and using the KVM_GET/SET_ONE_REG API to set.
Yes, if this is possible, why would you want something more?
we will use this method.
quoted
(2)using existed in-kernel API "kvm_inject_dabt" to inject through IOCTL command from Qemu.
I'm not really going to consider this, because "use internal API from
userspace" doesn't work.
So this should be:
(2) Introduce a new API to do X.
you can ignore the second method, now we will not use it.
I still think you know what my preference is; use the existing API if at
all possible.
Thanks,
-Christoffer
.
From: James Morse <james.morse@arm.com> Date: 2017-05-12 17:26:17
Hi gengdongjiu,
On 10/05/17 09:44, gengdongjiu wrote:
On 2017/5/9 1:28, James Morse wrote:
quoted
quoted
quoted
(hwpoison for KVM is a corner case as Qemu's memory effectively has two users,
Qemu and KVM. This isn't the example of how user-space gets signalled.)
KVM creates guests as if they were additional users of Qemu's memory. The code
in mm/memory-failure.c may find that Qemu didn't have the affected page mapped
to user-space - but it may have been in use by stage2.
The KVM+SIGBUS patch hides this difference, meaning Qemu gets a signal when the
guest touches the hwpoison page as if Qemu had touched the page itself.
Signals from KVM is a corner case, for firmware-first decisions should happen in
the APEI code based on CPER records.
quoted
quoted
If so, how the KVM handle the SEA type other than hwpoison?
quoted
To deliver to a guest? It shouldn't have to know, user space should use a KVM
API to drive this.
When received from hardware? It shouldn't have to care, these things should be
passed into the APEI code for handling. KVM just needs to put the host registers
back.
Recently I confirmed with the hardware team. they said almost all the SEA errors have the
Poison flag, so may be there is no need to consider other SEA errors other than hwPoison.
only consider SEA hwpoison errors can be enough.
We should be careful here, by hwpoison I meant the Linux feature.
From Documentation/vm/hwpoison.txt:
Upcoming Intel CPUs have support for recovering from some memory errors
(``MCA recovery''). This requires the OS to declare a page "poisoned",
kill the processes associated with it and avoid using it in the future.
We were talking about KVM's reaction to 'the OS declaring a page poisoned'.
Lets try to call this one memory-failure, as that is its Kconfig name. (now I
understand why we've been confusing each other!)
Your hwpoison looks like something the CPU reports in the ERR<n>STATUS registers
(4.6.10 of DDI0587). This is something firmware should read, then describe to
the OS via CPER records. Depending on these CPER records linux may invoke its
memory-failure code.
quoted
quoted
injection a SEA is no more than setting some registers: elr_el1, PC,
PSTATE, SPSR_el1, far_el1, esr_el1
I seen this KVM API do the same thing as Qemu. do you found call this
API will have issue and necessary to choose another ESR value?
Should we let user-space pick the ESR to deliver to the guest? Yes, letting
user-space specify the ESR gives the most flexibility to do something clever in
the future. An obvious choice for SEA is between the external-abort and 'parity
or ECC error' codes. If we tell user-space which of these happened (I don't
think Linux does today) then Qemu can relay that information to the guest.
may be the ESR is delivered by the KVM.
(1) guest OS EL0 happen SEA due to hwpoison
(2) CPU traps to EL3 firmware, and update the ESR_EL3
(3) the EL3 firmware copies the ESR_EL3 to ESR_EL2
(4) then jump to EL2 hypervisor, hypervisor uses the ESR_EL2 to inject the SEA.
May be the esr_el2 can provide the accurate error information.
or do you think user-space specify the ESR instead of esr_el2 is better?
I think the severity needs to be considered as the notification is handled by
each exception level. There are cases where it will need to be upgraded from
'contained' to 'uncontained'. (more discussion on another part of the thread).
Thanks,
James
2017-05-13 1:25 GMT+08:00, James Morse [off-list ref]:
Hi gengdongjiu,
On 10/05/17 09:44, gengdongjiu wrote:
quoted
On 2017/5/9 1:28, James Morse wrote:
quoted
quoted
quoted
(hwpoison for KVM is a corner case as Qemu's memory effectively has two
users,
Qemu and KVM. This isn't the example of how user-space gets
signalled.)
KVM creates guests as if they were additional users of Qemu's memory. The
code
in mm/memory-failure.c may find that Qemu didn't have the affected page
mapped
to user-space - but it may have been in use by stage2.
The KVM+SIGBUS patch hides this difference, meaning Qemu gets a signal
when the
guest touches the hwpoison page as if Qemu had touched the page itself.
Signals from KVM is a corner case, for firmware-first decisions should
happen in
the APEI code based on CPER records.
quoted
quoted
quoted
If so, how the KVM handle the SEA type other than hwpoison?
quoted
quoted
To deliver to a guest? It shouldn't have to know, user space should use a
KVM
API to drive this.
When received from hardware? It shouldn't have to care, these things
should be
passed into the APEI code for handling. KVM just needs to put the host
registers
back.
quoted
Recently I confirmed with the hardware team. they said almost all the SEA
errors have the
Poison flag, so may be there is no need to consider other SEA errors other
than hwPoison.
only consider SEA hwpoison errors can be enough.
We should be careful here, by hwpoison I meant the Linux feature.
From Documentation/vm/hwpoison.txt:
quoted
Upcoming Intel CPUs have support for recovering from some memory errors
(``MCA recovery''). This requires the OS to declare a page "poisoned",
kill the processes associated with it and avoid using it in the future.
We were talking about KVM's reaction to 'the OS declaring a page poisoned'.
Lets try to call this one memory-failure, as that is its Kconfig name. (now
I
understand why we've been confusing each other!)
Your hwpoison looks like something the CPU reports in the ERR<n>STATUS
registers
(4.6.10 of DDI0587). This is something firmware should read, then describe
to
the OS via CPER records. Depending on these CPER records linux may invoke
its
memory-failure code.
yes
quoted
quoted
quoted
injection a SEA is no more than setting some registers: elr_el1, PC,
PSTATE, SPSR_el1, far_el1, esr_el1
I seen this KVM API do the same thing as Qemu. do you found call this
API will have issue and necessary to choose another ESR value?
Should we let user-space pick the ESR to deliver to the guest? Yes,
letting
user-space specify the ESR gives the most flexibility to do something
clever in
the future. An obvious choice for SEA is between the external-abort and
'parity
or ECC error' codes. If we tell user-space which of these happened (I
don't
think Linux does today) then Qemu can relay that information to the
guest.
quoted
may be the ESR is delivered by the KVM.
(1) guest OS EL0 happen SEA due to hwpoison
(2) CPU traps to EL3 firmware, and update the ESR_EL3
(3) the EL3 firmware copies the ESR_EL3 to ESR_EL2
(4) then jump to EL2 hypervisor, hypervisor uses the ESR_EL2 to inject the
SEA.
May be the esr_el2 can provide the accurate error information.
or do you think user-space specify the ESR instead of esr_el2 is better?
I think the severity needs to be considered as the notification is handled
by
each exception level. There are cases where it will need to be upgraded
from
'contained' to 'uncontained'. (more discussion on another part of the
thread).