Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1356507 > unrolled thread
| Started by | Andy Lutomirski <luto@kernel.org> |
|---|---|
| First post | 2016-03-12 19:10 +0100 |
| Last post | 2016-03-14 15:40 +0100 |
| Articles | 6 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH v4 0/5] [PATCH v3 0/5] Improve non-"safe" MSR access failure handling Andy Lutomirski <luto@kernel.org> - 2016-03-12 19:10 +0100
[PATCH v4 1/5] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks Andy Lutomirski <luto@kernel.org> - 2016-03-12 19:20 +0100
Re: [PATCH v4 1/5] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks Borislav Petkov <bp@alien8.de> - 2016-03-14 13:00 +0100
Re: [PATCH v4 1/5] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks Andy Lutomirski <luto@amacapital.net> - 2016-03-14 18:10 +0100
[PATCH v4 5/5] x86/msr: Set the return value to zero when native_rdmsr_safe fails Andy Lutomirski <luto@kernel.org> - 2016-03-12 19:20 +0100
Re: [Xen-devel] [PATCH v4 0/5] [PATCH v3 0/5] Improve non-"safe" MSR access failure handling Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-03-14 15:40 +0100
| From | Andy Lutomirski <luto@kernel.org> |
|---|---|
| Date | 2016-03-12 19:10 +0100 |
| Subject | [PATCH v4 0/5] [PATCH v3 0/5] Improve non-"safe" MSR access failure handling |
| Message-ID | <rbW9c-1Em-5@gated-at.bofh.it> |
Setting CONFIG_PARAVIRT=y has an unintended side effect: it silently
turns all rdmsr and wrmsr operations into the safe variants without
any checks that the operations actually succeed.
With CONFIG_PARAVIRT=n, unchecked MSR failures OOPS and probably
cause boot to fail if they happen before init starts.
Neither behavior is very good, and it's particularly unfortunate that
the behavior changes depending on CONFIG_PARAVIRT.
In particular, KVM guests might be unwittingly depending on the
PARAVIRT=y behavior because CONFIG_KVM_GUEST currently depends on
CONFIG_PARAVIRT, and, because accesses in that case are completely
unchecked, we wouldn't even see a warning.
This series changes the native behavior, regardless of
CONFIG_PARAVIRT. A non-"safe" MSR failure will give an informative
warning once and will be fixed up -- native_read_msr will return
zero, and both reads and writes will continue where they left off.
If panic_on_oops is set, they will still OOPS and panic.
By using the shiny new custom exception handler infrastructure,
there should be no overhead on the success paths.
I didn't change the behavior on Xen, but, with this series applied,
it would be straightforward for the Xen maintainers to make the
corresponding change -- knowledge of whether the access is "safe" is
now propagated into the pvops.
Doing this is probably a prerequisite to sanely decoupling
CONFIG_KVM_GUEST and CONFIG_PARAVIRT, which would probably make
Arjan and the rest of the Clear Containers people happy :)
There's also room to reduce the code size of the "safe" variants
using custom exception handlers in the future.
Changes from v3:
- WARN_ONCE instead of WARN (Ingo)
- In the warning text, s/unsafe/unchecked/ (Ingo, sort of)
Changes from earlier versions: lots of changes!
Andy Lutomirski (5):
x86/paravirt: Add _safe to the read_msr and write_msr PV hooks
x86/msr: Carry on after a non-"safe" MSR access fails without
!panic_on_oops
x86/paravirt: Add paravirt_{read,write}_msr
x86/paravirt: Make "unsafe" MSR accesses unsafe even if PARAVIRT=y
x86/msr: Set the return value to zero when native_rdmsr_safe fails
arch/x86/include/asm/msr.h | 20 ++++++++++++----
arch/x86/include/asm/paravirt.h | 45 +++++++++++++++++++++--------------
arch/x86/include/asm/paravirt_types.h | 14 +++++++----
arch/x86/kernel/paravirt.c | 6 +++--
arch/x86/mm/extable.c | 33 +++++++++++++++++++++++++
arch/x86/xen/enlighten.c | 27 +++++++++++++++++++--
6 files changed, 114 insertions(+), 31 deletions(-)
--
2.5.0
[toc] | [next] | [standalone]
| From | Andy Lutomirski <luto@kernel.org> |
|---|---|
| Date | 2016-03-12 19:20 +0100 |
| Subject | [PATCH v4 1/5] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks |
| Message-ID | <rbWiS-1L9-11@gated-at.bofh.it> |
| In reply to | #1356507 |
These hooks match the _safe variants, so name them accordingly.
This will make room for unsafe PV hooks.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
arch/x86/include/asm/paravirt.h | 33 +++++++++++++++++----------------
arch/x86/include/asm/paravirt_types.h | 8 ++++----
arch/x86/kernel/paravirt.c | 4 ++--
arch/x86/xen/enlighten.c | 4 ++--
4 files changed, 25 insertions(+), 24 deletions(-)
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index f6192502149e..2e49228ed9a3 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -129,34 +129,35 @@ static inline void wbinvd(void)
#define get_kernel_rpl() (pv_info.kernel_rpl)
-static inline u64 paravirt_read_msr(unsigned msr, int *err)
+static inline u64 paravirt_read_msr_safe(unsigned msr, int *err)
{
- return PVOP_CALL2(u64, pv_cpu_ops.read_msr, msr, err);
+ return PVOP_CALL2(u64, pv_cpu_ops.read_msr_safe, msr, err);
}
-static inline int paravirt_write_msr(unsigned msr, unsigned low, unsigned high)
+static inline int paravirt_write_msr_safe(unsigned msr,
+ unsigned low, unsigned high)
{
- return PVOP_CALL3(int, pv_cpu_ops.write_msr, msr, low, high);
+ return PVOP_CALL3(int, pv_cpu_ops.write_msr_safe, msr, low, high);
}
/* These should all do BUG_ON(_err), but our headers are too tangled. */
#define rdmsr(msr, val1, val2) \
do { \
int _err; \
- u64 _l = paravirt_read_msr(msr, &_err); \
+ u64 _l = paravirt_read_msr_safe(msr, &_err); \
val1 = (u32)_l; \
val2 = _l >> 32; \
} while (0)
#define wrmsr(msr, val1, val2) \
do { \
- paravirt_write_msr(msr, val1, val2); \
+ paravirt_write_msr_safe(msr, val1, val2); \
} while (0)
#define rdmsrl(msr, val) \
do { \
int _err; \
- val = paravirt_read_msr(msr, &_err); \
+ val = paravirt_read_msr_safe(msr, &_err); \
} while (0)
static inline void wrmsrl(unsigned msr, u64 val)
@@ -164,23 +165,23 @@ static inline void wrmsrl(unsigned msr, u64 val)
wrmsr(msr, (u32)val, (u32)(val>>32));
}
-#define wrmsr_safe(msr, a, b) paravirt_write_msr(msr, a, b)
+#define wrmsr_safe(msr, a, b) paravirt_write_msr_safe(msr, a, b)
/* rdmsr with exception handling */
-#define rdmsr_safe(msr, a, b) \
-({ \
- int _err; \
- u64 _l = paravirt_read_msr(msr, &_err); \
- (*a) = (u32)_l; \
- (*b) = _l >> 32; \
- _err; \
+#define rdmsr_safe(msr, a, b) \
+({ \
+ int _err; \
+ u64 _l = paravirt_read_msr_safe(msr, &_err); \
+ (*a) = (u32)_l; \
+ (*b) = _l >> 32; \
+ _err; \
})
static inline int rdmsrl_safe(unsigned msr, unsigned long long *p)
{
int err;
- *p = paravirt_read_msr(msr, &err);
+ *p = paravirt_read_msr_safe(msr, &err);
return err;
}
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 77db5616a473..5a06cccd36f0 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -155,10 +155,10 @@ struct pv_cpu_ops {
void (*cpuid)(unsigned int *eax, unsigned int *ebx,
unsigned int *ecx, unsigned int *edx);
- /* MSR, PMC and TSR operations.
- err = 0/-EFAULT. wrmsr returns 0/-EFAULT. */
- u64 (*read_msr)(unsigned int msr, int *err);
- int (*write_msr)(unsigned int msr, unsigned low, unsigned high);
+ /* MSR operations.
+ err = 0/-EIO. wrmsr returns 0/-EIO. */
+ u64 (*read_msr_safe)(unsigned int msr, int *err);
+ int (*write_msr_safe)(unsigned int msr, unsigned low, unsigned high);
u64 (*read_pmc)(int counter);
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index f08ac28b8136..8aad95478ae5 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -339,8 +339,8 @@ __visible struct pv_cpu_ops pv_cpu_ops = {
.write_cr8 = native_write_cr8,
#endif
.wbinvd = native_wbinvd,
- .read_msr = native_read_msr_safe,
- .write_msr = native_write_msr_safe,
+ .read_msr_safe = native_read_msr_safe,
+ .write_msr_safe = native_write_msr_safe,
.read_pmc = native_read_pmc,
.load_tr_desc = native_load_tr_desc,
.set_ldt = native_set_ldt,
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index d09e4c9d7cc5..ff2df20d0067 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1222,8 +1222,8 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
.wbinvd = native_wbinvd,
- .read_msr = xen_read_msr_safe,
- .write_msr = xen_write_msr_safe,
+ .read_msr_safe = xen_read_msr_safe,
+ .write_msr_safe = xen_write_msr_safe,
.read_pmc = xen_read_pmc,
--
2.5.0
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2016-03-14 13:00 +0100 |
| Subject | Re: [PATCH v4 1/5] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks |
| Message-ID | <rczke-3fH-9@gated-at.bofh.it> |
| In reply to | #1356514 |
On Sat, Mar 12, 2016 at 10:08:48AM -0800, Andy Lutomirski wrote:
> These hooks match the _safe variants, so name them accordingly.
> This will make room for unsafe PV hooks.
>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
> arch/x86/include/asm/paravirt.h | 33 +++++++++++++++++----------------
> arch/x86/include/asm/paravirt_types.h | 8 ++++----
> arch/x86/kernel/paravirt.c | 4 ++--
> arch/x86/xen/enlighten.c | 4 ++--
> 4 files changed, 25 insertions(+), 24 deletions(-)
>
> diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
> index f6192502149e..2e49228ed9a3 100644
> --- a/arch/x86/include/asm/paravirt.h
> +++ b/arch/x86/include/asm/paravirt.h
> @@ -129,34 +129,35 @@ static inline void wbinvd(void)
>
> #define get_kernel_rpl() (pv_info.kernel_rpl)
>
> -static inline u64 paravirt_read_msr(unsigned msr, int *err)
> +static inline u64 paravirt_read_msr_safe(unsigned msr, int *err)
> {
> - return PVOP_CALL2(u64, pv_cpu_ops.read_msr, msr, err);
> + return PVOP_CALL2(u64, pv_cpu_ops.read_msr_safe, msr, err);
> }
>
> -static inline int paravirt_write_msr(unsigned msr, unsigned low, unsigned high)
> +static inline int paravirt_write_msr_safe(unsigned msr,
> + unsigned low, unsigned high)
> {
> - return PVOP_CALL3(int, pv_cpu_ops.write_msr, msr, low, high);
> + return PVOP_CALL3(int, pv_cpu_ops.write_msr_safe, msr, low, high);
> }
>
> /* These should all do BUG_ON(_err), but our headers are too tangled. */
> #define rdmsr(msr, val1, val2) \
> do { \
> int _err; \
> - u64 _l = paravirt_read_msr(msr, &_err); \
> + u64 _l = paravirt_read_msr_safe(msr, &_err); \
> val1 = (u32)_l; \
> val2 = _l >> 32; \
> } while (0)
>
> #define wrmsr(msr, val1, val2) \
> do { \
> - paravirt_write_msr(msr, val1, val2); \
> + paravirt_write_msr_safe(msr, val1, val2); \
> } while (0)
>
> #define rdmsrl(msr, val) \
> do { \
> int _err; \
> - val = paravirt_read_msr(msr, &_err); \
> + val = paravirt_read_msr_safe(msr, &_err); \
> } while (0)
>
> static inline void wrmsrl(unsigned msr, u64 val)
> @@ -164,23 +165,23 @@ static inline void wrmsrl(unsigned msr, u64 val)
> wrmsr(msr, (u32)val, (u32)(val>>32));
> }
>
> -#define wrmsr_safe(msr, a, b) paravirt_write_msr(msr, a, b)
> +#define wrmsr_safe(msr, a, b) paravirt_write_msr_safe(msr, a, b)
>
> /* rdmsr with exception handling */
> -#define rdmsr_safe(msr, a, b) \
> -({ \
> - int _err; \
> - u64 _l = paravirt_read_msr(msr, &_err); \
> - (*a) = (u32)_l; \
> - (*b) = _l >> 32; \
> - _err; \
> +#define rdmsr_safe(msr, a, b) \
> +({ \
> + int _err; \
> + u64 _l = paravirt_read_msr_safe(msr, &_err); \
> + (*a) = (u32)_l; \
> + (*b) = _l >> 32; \
> + _err; \
> })
>
> static inline int rdmsrl_safe(unsigned msr, unsigned long long *p)
> {
> int err;
>
> - *p = paravirt_read_msr(msr, &err);
> + *p = paravirt_read_msr_safe(msr, &err);
> return err;
> }
>
> diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
> index 77db5616a473..5a06cccd36f0 100644
> --- a/arch/x86/include/asm/paravirt_types.h
> +++ b/arch/x86/include/asm/paravirt_types.h
> @@ -155,10 +155,10 @@ struct pv_cpu_ops {
> void (*cpuid)(unsigned int *eax, unsigned int *ebx,
> unsigned int *ecx, unsigned int *edx);
>
> - /* MSR, PMC and TSR operations.
> - err = 0/-EFAULT. wrmsr returns 0/-EFAULT. */
> - u64 (*read_msr)(unsigned int msr, int *err);
> - int (*write_msr)(unsigned int msr, unsigned low, unsigned high);
> + /* MSR operations.
> + err = 0/-EIO. wrmsr returns 0/-EIO. */
Please fix the comment format while you're touching this :
/*
* A sentence.
* Another sentence.
*/
Other than that:
Acked-by: Borislav Petkov <bp@suse.de>
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2016-03-14 18:10 +0100 |
| Subject | Re: [PATCH v4 1/5] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks |
| Message-ID | <rcEaf-6JJ-19@gated-at.bofh.it> |
| In reply to | #1357182 |
On Mon, Mar 14, 2016 at 4:57 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Sat, Mar 12, 2016 at 10:08:48AM -0800, Andy Lutomirski wrote:
>> These hooks match the _safe variants, so name them accordingly.
>> This will make room for unsafe PV hooks.
>>
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> ---
>> arch/x86/include/asm/paravirt.h | 33 +++++++++++++++++----------------
>> arch/x86/include/asm/paravirt_types.h | 8 ++++----
>> arch/x86/kernel/paravirt.c | 4 ++--
>> arch/x86/xen/enlighten.c | 4 ++--
>> 4 files changed, 25 insertions(+), 24 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
>> index f6192502149e..2e49228ed9a3 100644
>> --- a/arch/x86/include/asm/paravirt.h
>> +++ b/arch/x86/include/asm/paravirt.h
>> @@ -129,34 +129,35 @@ static inline void wbinvd(void)
>>
>> #define get_kernel_rpl() (pv_info.kernel_rpl)
>>
>> -static inline u64 paravirt_read_msr(unsigned msr, int *err)
>> +static inline u64 paravirt_read_msr_safe(unsigned msr, int *err)
>> {
>> - return PVOP_CALL2(u64, pv_cpu_ops.read_msr, msr, err);
>> + return PVOP_CALL2(u64, pv_cpu_ops.read_msr_safe, msr, err);
>> }
>>
>> -static inline int paravirt_write_msr(unsigned msr, unsigned low, unsigned high)
>> +static inline int paravirt_write_msr_safe(unsigned msr,
>> + unsigned low, unsigned high)
>> {
>> - return PVOP_CALL3(int, pv_cpu_ops.write_msr, msr, low, high);
>> + return PVOP_CALL3(int, pv_cpu_ops.write_msr_safe, msr, low, high);
>> }
>>
>> /* These should all do BUG_ON(_err), but our headers are too tangled. */
>> #define rdmsr(msr, val1, val2) \
>> do { \
>> int _err; \
>> - u64 _l = paravirt_read_msr(msr, &_err); \
>> + u64 _l = paravirt_read_msr_safe(msr, &_err); \
>> val1 = (u32)_l; \
>> val2 = _l >> 32; \
>> } while (0)
>>
>> #define wrmsr(msr, val1, val2) \
>> do { \
>> - paravirt_write_msr(msr, val1, val2); \
>> + paravirt_write_msr_safe(msr, val1, val2); \
>> } while (0)
>>
>> #define rdmsrl(msr, val) \
>> do { \
>> int _err; \
>> - val = paravirt_read_msr(msr, &_err); \
>> + val = paravirt_read_msr_safe(msr, &_err); \
>> } while (0)
>>
>> static inline void wrmsrl(unsigned msr, u64 val)
>> @@ -164,23 +165,23 @@ static inline void wrmsrl(unsigned msr, u64 val)
>> wrmsr(msr, (u32)val, (u32)(val>>32));
>> }
>>
>> -#define wrmsr_safe(msr, a, b) paravirt_write_msr(msr, a, b)
>> +#define wrmsr_safe(msr, a, b) paravirt_write_msr_safe(msr, a, b)
>>
>> /* rdmsr with exception handling */
>> -#define rdmsr_safe(msr, a, b) \
>> -({ \
>> - int _err; \
>> - u64 _l = paravirt_read_msr(msr, &_err); \
>> - (*a) = (u32)_l; \
>> - (*b) = _l >> 32; \
>> - _err; \
>> +#define rdmsr_safe(msr, a, b) \
>> +({ \
>> + int _err; \
>> + u64 _l = paravirt_read_msr_safe(msr, &_err); \
>> + (*a) = (u32)_l; \
>> + (*b) = _l >> 32; \
>> + _err; \
>> })
>>
>> static inline int rdmsrl_safe(unsigned msr, unsigned long long *p)
>> {
>> int err;
>>
>> - *p = paravirt_read_msr(msr, &err);
>> + *p = paravirt_read_msr_safe(msr, &err);
>> return err;
>> }
>>
>> diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
>> index 77db5616a473..5a06cccd36f0 100644
>> --- a/arch/x86/include/asm/paravirt_types.h
>> +++ b/arch/x86/include/asm/paravirt_types.h
>> @@ -155,10 +155,10 @@ struct pv_cpu_ops {
>> void (*cpuid)(unsigned int *eax, unsigned int *ebx,
>> unsigned int *ecx, unsigned int *edx);
>>
>> - /* MSR, PMC and TSR operations.
>> - err = 0/-EFAULT. wrmsr returns 0/-EFAULT. */
>> - u64 (*read_msr)(unsigned int msr, int *err);
>> - int (*write_msr)(unsigned int msr, unsigned low, unsigned high);
>> + /* MSR operations.
>> + err = 0/-EIO. wrmsr returns 0/-EIO. */
>
> Please fix the comment format while you're touching this :
>
> /*
> * A sentence.
> * Another sentence.
> */
>
> Other than that:
>
> Acked-by: Borislav Petkov <bp@suse.de>
I fixed this in "x86/paravirt: Add paravirt_{read,write}_msr" later in
the series. Is that good enough?
--Andy
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@kernel.org> |
|---|---|
| Date | 2016-03-12 19:20 +0100 |
| Subject | [PATCH v4 5/5] x86/msr: Set the return value to zero when native_rdmsr_safe fails |
| Message-ID | <rbWiS-1L9-13@gated-at.bofh.it> |
| In reply to | #1356507 |
This will cause unchecked native_rdmsr_safe failures to return
deterministic results.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
arch/x86/include/asm/msr.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
index 13da359881d7..e97e79f8a22b 100644
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -109,7 +109,10 @@ static inline unsigned long long native_read_msr_safe(unsigned int msr,
asm volatile("2: rdmsr ; xor %[err],%[err]\n"
"1:\n\t"
".section .fixup,\"ax\"\n\t"
- "3: mov %[fault],%[err] ; jmp 1b\n\t"
+ "3: mov %[fault],%[err]\n\t"
+ "xorl %%eax, %%eax\n\t"
+ "xorl %%edx, %%edx\n\t"
+ "jmp 1b\n\t"
".previous\n\t"
_ASM_EXTABLE(2b, 3b)
: [err] "=r" (*err), EAX_EDX_RET(val, low, high)
--
2.5.0
[toc] | [prev] | [next] | [standalone]
| From | Boris Ostrovsky <boris.ostrovsky@oracle.com> |
|---|---|
| Date | 2016-03-14 15:40 +0100 |
| Subject | Re: [Xen-devel] [PATCH v4 0/5] [PATCH v3 0/5] Improve non-"safe" MSR access failure handling |
| Message-ID | <rcBP4-4Xz-35@gated-at.bofh.it> |
| In reply to | #1356507 |
On 03/12/2016 01:08 PM, Andy Lutomirski wrote:
> Setting CONFIG_PARAVIRT=y has an unintended side effect: it silently
> turns all rdmsr and wrmsr operations into the safe variants without
> any checks that the operations actually succeed.
>
> With CONFIG_PARAVIRT=n, unchecked MSR failures OOPS and probably
> cause boot to fail if they happen before init starts.
>
> Neither behavior is very good, and it's particularly unfortunate that
> the behavior changes depending on CONFIG_PARAVIRT.
>
> In particular, KVM guests might be unwittingly depending on the
> PARAVIRT=y behavior because CONFIG_KVM_GUEST currently depends on
> CONFIG_PARAVIRT, and, because accesses in that case are completely
> unchecked, we wouldn't even see a warning.
>
> This series changes the native behavior, regardless of
> CONFIG_PARAVIRT. A non-"safe" MSR failure will give an informative
> warning once and will be fixed up -- native_read_msr will return
> zero, and both reads and writes will continue where they left off.
>
> If panic_on_oops is set, they will still OOPS and panic.
>
> By using the shiny new custom exception handler infrastructure,
> there should be no overhead on the success paths.
>
> I didn't change the behavior on Xen, but, with this series applied,
> it would be straightforward for the Xen maintainers to make the
> corresponding change -- knowledge of whether the access is "safe" is
> now propagated into the pvops.
>
> Doing this is probably a prerequisite to sanely decoupling
> CONFIG_KVM_GUEST and CONFIG_PARAVIRT, which would probably make
> Arjan and the rest of the Clear Containers people happy :)
>
> There's also room to reduce the code size of the "safe" variants
> using custom exception handlers in the future.
>
> Changes from v3:
> - WARN_ONCE instead of WARN (Ingo)
> - In the warning text, s/unsafe/unchecked/ (Ingo, sort of)
>
> Changes from earlier versions: lots of changes!
>
> Andy Lutomirski (5):
> x86/paravirt: Add _safe to the read_msr and write_msr PV hooks
> x86/msr: Carry on after a non-"safe" MSR access fails without
> !panic_on_oops
> x86/paravirt: Add paravirt_{read,write}_msr
> x86/paravirt: Make "unsafe" MSR accesses unsafe even if PARAVIRT=y
> x86/msr: Set the return value to zero when native_rdmsr_safe fails
>
> arch/x86/include/asm/msr.h | 20 ++++++++++++----
> arch/x86/include/asm/paravirt.h | 45 +++++++++++++++++++++--------------
> arch/x86/include/asm/paravirt_types.h | 14 +++++++----
> arch/x86/kernel/paravirt.c | 6 +++--
> arch/x86/mm/extable.c | 33 +++++++++++++++++++++++++
> arch/x86/xen/enlighten.c | 27 +++++++++++++++++++--
> 6 files changed, 114 insertions(+), 31 deletions(-)
>
I don't see any issues as far as Xen is concerned but let me run this
through our nightly. I'll wait for the next version (which I think you
might have based on the comments). Please copy me.
-boris
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web