On 03/12/2013 02:40:39 AM, Jia Hongtao-B38951 wrote:

=20
=20

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Saturday, March 09, 2013 8:49 AM
To: Jia Hongtao-B38951
Cc: Wood Scott-B07421; David Laight; linuxppc-dev@lists.ozlabs.org;
Stuart Yoder
Subject: Re: [PATCH V4] powerpc/85xx: Add machine check handler to =20

fix

quoted

PCIe erratum on mpc85xx

On 03/08/2013 02:01:46 AM, Jia Hongtao-B38951 wrote:

quoted

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Friday, March 08, 2013 12:38 AM
To: Jia Hongtao-B38951
Cc: David Laight; Wood Scott-B07421; =20

linuxppc-dev@lists.ozlabs.org;

quoted

quoted

quoted

Stuart Yoder
Subject: Re: [PATCH V4] powerpc/85xx: Add machine check handler =20

to

quoted

quoted

fix

quoted

PCIe erratum on mpc85xx

On 03/07/2013 02:06:05 AM, Jia Hongtao-B38951 wrote:

quoted

Here is the ideas from Scott:
"

quoted

+	if (is_in_pci_mem_space(addr)) {
+		inst =3D *(unsigned int *)regs->nip;

Be careful about taking a fault here.  A simple TLB miss =20

should be

quoted

quoted

quoted

quoted

safe given that we shouldn't be accessing PCIe in the middle =20

of

quoted

quoted

quoted

quoted

exception code, but what if the mapping has gone away (e.g. a
userspace driver had its code munmap()ed or swapped out)?  =20

What if

quoted

quoted

quoted

quoted

permissions allow execute but not read (not sure if Linux will

allow

quoted

quoted

this, but the hardware does)?

What if it happened in a KVM guest?  You can't access guest

addresses

quoted

quoted

directly.
"

That means you need to be careful about how you read the

instruction, not

quoted

that you shouldn't do it at all.

-Scott

I agree.

Do you have a more secure way to get the instruction?
Or what should be done to avoid permission break issue?

probe_kernel_address() should take care of userspace issues.  As for
KVM, if you see MSR_GS set, bail out and don't apply the workaround.
Let KVM/QEMU deal with it as it wishes (e.g. reflect to the guest =20

and

quoted

let its machine check handler do the skipping).  On PR-mode KVM =20

(e.g.

quoted

on e500v2-based chips) there is no MSR_GS and it just looks like
userspace code -- for now just pretend it is user mode.

-Scott

=20
Hi Scott,
=20
Is that OK if I use the following code?
=20
	u32 inst;
	int ret;
=20
	if (is_in_pci_mem_space(addr)) {
		if (!user_mode(regs)) {
			ret =3D probe_kernel_address(regs->nip, inst);

Hmm, seems there's no probe_user_address() -- for userspace we =20
basically want the same thing minus the KERNEL_DS.  See =20
arch/powerpc/perf/callchain.c for an example.

You also need to skip this if (regs->msr & MSR_GS) as I mentioned above.

			if (!ret) {
				rd =3D get_rt(inst);
				regs->gpr[rd] =3D 0xffffffff;
			}

Check whether the instruction is a load, as David pointed out.  Also =20
check the size of the load, whether it was load with update =20
instruction, etc.

-Scott=