Re: Lock up when faking MMIO read[bwl] on some machines [WAS: Faking MMIO ops? Fooling a driver]

["Rafał Miłecki" <zajec5@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2128 bytes --]

W dniu 18 czerwca 2011 14:03 użytkownik Pekka Paalanen <pq@iki.fi> napisał:
> Maybe the driver is doing a 16-bit wide access, and happens to
> store something else in the other 16/48 bits of RAX?

OK, attached is updated version of my patch. I think we can get some
clue from dmesgs with this patch applied.


My system (working):
[   74.472502] mmiotrace: ZAJEC: overwriting 0x27 with 0xFFFF
[   74.472511] mmiotrace: [ZAJEC] opcode is 0x8B
[   74.472515] mmiotrace: [ZAJEC] prf info: shorted:1; enlarged:0, rexr:0, rex:0
[   74.472517] mmiotrace: [ZAJEC] register is 0x0
[   74.472520] mmiotrace: [ZAJEC] overwritting 2-byte value 0x0514 with 0xFFFF
[   74.472523] mmiotrace: [ZAJEC] overwritting resulted in 0xFFFF

[   74.487081] mmiotrace: ZAJEC: overwriting 0x20 with 0xFFFF
[   74.487086] mmiotrace: [ZAJEC] opcode is 0x8B
[   74.487089] mmiotrace: [ZAJEC] prf info: shorted:1; enlarged:0, rexr:0, rex:0
[   74.487092] mmiotrace: [ZAJEC] register is 0x0
[   74.487095] mmiotrace: [ZAJEC] overwritting 2-byte value 0x427E with 0xFFFF
[   74.487097] mmiotrace: [ZAJEC] overwritting resulted in 0xFFFF


MacBook (with real overwritting commenet out!):
[  228.248715] mmiotrace: ZAJEC: overwriting 0x810 with 0xFFFF
[  228.254227] mmiotrace: [ZAJEC] opcode is 0xB70F
[  228.259784] mmiotrace: [ZAJEC] prf info: shorted:0; enlarged:0, rexr:0, rex:0
[  228.265399] mmiotrace: [ZAJEC] register is 0x0
[  228.270955] mmiotrace: [ZAJEC] overwritting 4-byte value 0x00000000
with 0xFFFF
[  228.276597] mmiotrace: [ZAJEC] overwritting resulted in 0x00000000

[  228.284284] mmiotrace: ZAJEC: overwriting 0x810 with 0xFFFF
[  228.289818] mmiotrace: [ZAJEC] opcode is 0xB70F
[  228.295250] mmiotrace: [ZAJEC] prf info: shorted:0; enlarged:0, rexr:0, rex:0
[  228.300838] mmiotrace: [ZAJEC] register is 0x0
[  228.306339] mmiotrace: [ZAJEC] overwritting 4-byte value 0x00000000
with 0xFFFF
[  228.311905] mmiotrace: [ZAJEC] overwritting resulted in 0x00000000


It's 2-byte vs. 4-byte. I suspect this can be the source of our
problem. Writing u16 0xFFFF value as u32 write.

-- 
Rafał

[-- Attachment #2: mmio.debugging.patch --]
[-- Type: application/octet-stream, Size: 4443 bytes --]

diff --git a/arch/x86/mm/mmio-mod.c b/arch/x86/mm/mmio-mod.c
index 3adff7d..d5ac7da 100644
--- a/arch/x86/mm/mmio-mod.c
+++ b/arch/x86/mm/mmio-mod.c
@@ -206,6 +206,12 @@ static void pre(struct kmmio_probe *p, struct pt_regs *regs,
 	put_cpu_var(pf_reason);
 }
 
+#define MMIO_BASE			0xfaafc000
+#define B43_MMIO_PHY_CONTROL		0x3FC
+#define B43_MMIO_PHY_DATA		0x3FE
+
+static u16 broadcom_phy_addr;
+
 static void post(struct kmmio_probe *p, unsigned long condition,
 							struct pt_regs *regs)
 {
@@ -219,6 +225,19 @@ static void post(struct kmmio_probe *p, unsigned long condition,
 		BUG();
 	}
 
+	if (my_reason->type == REG_READ && my_trace->phys == MMIO_BASE + B43_MMIO_PHY_DATA) {
+		//pr_info("ZAJEC: read PHY 0x%X\n", broadcom_phy_addr);
+		switch (broadcom_phy_addr){
+		case 0x20:
+		case 0x22:
+		case 0x27:
+		case 0x810:
+			pr_info("ZAJEC: overwriting 0x%X with 0xFFFF\n", broadcom_phy_addr);
+			set_ins_reg_val(my_reason->ip, regs, 0xFFFF);
+			break;
+		}
+	}
+
 	switch (my_reason->type) {
 	case REG_READ:
 		my_trace->value = get_ins_reg_val(my_reason->ip, regs);
@@ -227,6 +246,11 @@ static void post(struct kmmio_probe *p, unsigned long condition,
 		break;
 	}
 
+	if (my_reason->type == REG_WRITE && my_trace->phys == MMIO_BASE + B43_MMIO_PHY_CONTROL) {
+		broadcom_phy_addr = my_trace->value;
+		//pr_info("ZAJEC: setting PHY addr to 0x%X\n", broadcom_phy_addr);
+	}
+
 	mmio_trace_rw(my_trace);
 	put_cpu_var(cpu_trace);
 	put_cpu_var(pf_reason);
diff --git a/arch/x86/mm/pf_in.c b/arch/x86/mm/pf_in.c
index 9f0614d..8519f69 100644
--- a/arch/x86/mm/pf_in.c
+++ b/arch/x86/mm/pf_in.c
@@ -461,6 +461,75 @@ err:
 	return 0;
 }
 
+void set_ins_reg_val(unsigned long ins_addr, struct pt_regs *regs, u32 val)
+{
+	unsigned int opcode;
+	int reg;
+	unsigned char *p;
+	struct prefix_bits prf;
+	int i;
+
+	p = (unsigned char *)ins_addr;
+	p += skip_prefix(p, &prf);
+	p += get_opcode(p, &opcode);
+	for (i = 0; i < ARRAY_SIZE(reg_rop); i++)
+		if (reg_rop[i] == opcode)
+			goto do_work;
+
+	for (i = 0; i < ARRAY_SIZE(reg_wop); i++)
+		if (reg_wop[i] == opcode)
+			goto do_work;
+
+	printk(KERN_ERR "mmiotrace: Not a register instruction, opcode "
+							"0x%02x\n", opcode);
+	return;
+
+do_work:
+	printk(KERN_INFO "mmiotrace: [ZAJEC] opcode is 0x%X\n", opcode);
+	printk(KERN_INFO "mmiotrace: [ZAJEC] prf info: shorted:%d; enlarged:%d, rexr:%d, rex:%d\n", prf.shorted, prf.enlarged, prf.rexr, prf.rex);
+	/* for STOS, source register is fixed */
+	if (opcode == 0xAA || opcode == 0xAB) {
+		reg = arg_AX;
+	} else {
+		unsigned char mod_rm = *p;
+		reg = ((mod_rm >> 3) & 0x7) | (prf.rexr << 3);
+	}
+	printk(KERN_INFO "mmiotrace: [ZAJEC] register is 0x%X\n", reg);
+	switch (get_ins_reg_width(ins_addr)) {
+	case 1:
+		printk(KERN_ERR "mmiotrace: [ZAJEC] unsupported width 1\n");
+		break;
+	case 2:
+		{
+		unsigned short *zptr = (unsigned short *)get_reg_w32(reg, regs);
+		printk(KERN_INFO "mmiotrace: [ZAJEC] overwritting 2-byte value 0x%04X with 0x%X\n", *(zptr), val);
+		*(zptr) = val;
+		printk(KERN_INFO "mmiotrace: [ZAJEC] overwritting resulted in 0x%04X\n", *(zptr));
+		}
+		break;
+	case 4:
+		{
+		unsigned int *zptr = (unsigned int *)get_reg_w32(reg, regs);
+		printk(KERN_INFO "mmiotrace: [ZAJEC] overwritting 4-byte value 0x%08X with 0x%X\n", *(zptr), val);
+		*(zptr) = val;
+		printk(KERN_INFO "mmiotrace: [ZAJEC] overwritting resulted in 0x%08X\n", *(zptr));
+		}
+		break;
+#ifdef __amd64__
+	case 8:
+		{
+		unsigned long *zptr = (unsigned long *)get_reg_w32(reg, regs);
+		printk(KERN_INFO "mmiotrace: [ZAJEC] overwritting 8-byte value 0x%016X with 0x%X\n", *(zptr), val);
+		*(zptr) = val;
+		printk(KERN_INFO "mmiotrace: [ZAJEC] overwritting resulted in 0x%016lX\n", *(zptr));
+		}
+		break;
+#endif
+	default:
+		printk(KERN_ERR "mmiotrace: [ZAJEC] Error width# %d\n", reg);
+	}
+}
+
 unsigned long get_ins_imm_val(unsigned long ins_addr)
 {
 	unsigned int opcode;
diff --git a/arch/x86/mm/pf_in.h b/arch/x86/mm/pf_in.h
index e05341a..90b43ff 100644
--- a/arch/x86/mm/pf_in.h
+++ b/arch/x86/mm/pf_in.h
@@ -34,6 +34,7 @@ enum reason_type {
 enum reason_type get_ins_type(unsigned long ins_addr);
 unsigned int get_ins_mem_width(unsigned long ins_addr);
 unsigned long get_ins_reg_val(unsigned long ins_addr, struct pt_regs *regs);
+void set_ins_reg_val(unsigned long ins_addr, struct pt_regs *regs, u32 val);
 unsigned long get_ins_imm_val(unsigned long ins_addr);
 
 #endif /* __PF_H_ */