Re: [PATCH 2/13] KVM: MIPS: Pass type of fault down to kvm_mips_map_page()

[James Hogan <james.hogan@imgtec.com>]

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

Hi,

On Tue, Jan 17, 2017 at 07:06:39PM +0800, jiang.biao2@zte.com.cn 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 #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]