Hi, On Tue, Jan 17, 2017 at 07:06:39PM +0800, jiang.biao2@xxxxxxxxxx wrote: > > kvm_mips_map_page() will need to know whether the fault was due to a > > read or a write in order to support dirty page tracking, > > KVM_CAP_SYNC_MMU, and read only memory regions, so get that information > > passed down to it via new bool write_fault arguments to various > > functions. > > Hi, James, > > > Maybe it's not good idea to add an argument and pass it down to various functions, it will , > > make the the interface more complicated, have more possibolity, and more difficult to test. > kvm_mips_map_page() needs to know is whether it is being triggered in response to a write, but it is abstracted and independent from what host exception triggered it. It is only really concerned with the GPA space. > > > > As far as I can see, what you need is to let kvm_mips_map_page() know the *reason*. > > would it be better to let kvm_mips_map_page() get *exccode* from the CP0 directly, or > > > provide any interface to get the fault reason? I presume you mean from the saved host cause register in the VCPU context (since intervening exceptions/interrupts will clobber the actual CP0 Cause register). It directly needs to know whether it can get away with a read-only mapping, and although it directly depends on a GVA segment, it doesn't necessarily relate to a memory access made by the guest. kvm_mips_map_page() is called via: - kvm_mips_handle_kseg0_tlb_fault() for faults in guest KSeg0 - kvm_mips_handle_mapped_seg_tlb_fault() for faults in guest TLB mapped segments From these functions: - kvm_trap_emul_handle_tlb_mod() (write_fault = true) in response to a write to a read-only page (exccode = MOD) - kvm_trap_emul_handle_tlb_miss() (write_fault = true or false) in response to a read or write when TLB mapping absent or invalid (exccode = TLBL/TLBS) - via the kvm_trap_emul_gva_fault() helper when KVM needs to directly access GVA space: - kvm_get_inst() (write_fault = false) when reading an instruction from guest RAM (when BadInstr/BadInstrP registers unavailable) which needs to be emulated, i.e. reserved instruction (exccode = RI), cop0 unusuable (exccode = CPU), MMIO load/store (TLBL/TLBS/MOD/ADEL/ADES), branch delay slot handling for MMIO load/store, or debug output for an unhandled exccode (exccode = ?) - kvm_mips_trans_replace() (write_fault = true) to write a replacement instruction into guest memory (mainly exccode = CPU) - kvm_mips_guest_cache_op() (write_fault = false) cache instruction emulation (exccode = CPU) So there is a many:many mapping from exccode to write_fault for these exccodes: - CPU (CoProcessor Unusable) could be reading instruction or servicing a CACHE instruction (write_fault = false) or replacing an instruction (write_fault = true). - MOD, TLBS, ADES could be the write itself (write_fault = true), or a read of the instruction triggering the exception or the prior branch instruction (write_fault = false). Cheers James
Attachment:
signature.asc
Description: Digital signature