[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 for-4.14] x86/vmx: use P2M_ALLOC in vmx_load_pdptrs instead of P2M_UNSHARE


  • To: Tamas K Lengyel <tamas.lengyel@xxxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Thu, 18 Jun 2020 17:46:28 +0200
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: Kevin Tian <kevin.tian@xxxxxxxxx>, Jun Nakajima <jun.nakajima@xxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Paul Durrant <paul@xxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Thu, 18 Jun 2020 15:46:40 +0000
  • Ironport-sdr: hhva6BU1rQl2VCDwwk4yuwkMXe2Wypada6Qo5g3E1C5DylH6c8+igtX3x48pwlmui30ekuvEmq eZhlbiXd5fv3vQscjL+gn5MMmOM1UJIxJXh5noXZglJ1izwktmdPtzMAISMyuwX7s4vwg1jrpc W5U2o3pCvpJgFOfmhW168H+P4/Td2yjpZv0OSCVBCZa8Kl9n5Fi4crHngXTi+ldRkaTbzihZA8 u3mChOrap0PzZFJBPYy8Ag4ifxsTk3G6cbPewM6AFUVHqFNb7HverkaHpuFZS3JUBYhsQSXvNK Raw=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Thu, Jun 18, 2020 at 07:39:04AM -0700, Tamas K Lengyel wrote:
> While forking VMs running a small RTOS system (Zephyr) a Xen crash has been
> observed due to a mm-lock order violation while copying the HVM CPU context
> from the parent. This issue has been identified to be due to
> hap_update_paging_modes first getting a lock on the gfn using get_gfn. This
> call also creates a shared entry in the fork's memory map for the cr3 gfn. The
> function later calls hap_update_cr3 while holding the paging_lock, which
> results in the lock-order violation in vmx_load_pdptrs when it tries to 
> unshare
> the above entry when it grabs the page with the P2M_UNSHARE flag set.
> 
> Since vmx_load_pdptrs only reads from the page its usage of P2M_UNSHARE was
> unnecessary to start with. Using P2M_ALLOC is the appropriate flag to ensure
> the p2m is properly populated.
> 
> Note that the lock order violation is avoided because before the paging_lock 
> is
> taken a lookup is performed with P2M_ALLOC that forks the page, thus the 
> second
> lookup in vmx_load_pdptrs succeeds without having to perform the fork. We keep
> P2M_ALLOC in vmx_load_pdptrs because there are code-paths leading up to it
> which don't take the paging_lock and that have no previous lookup. Currently 
> no
> other code-path exists leading there with the paging_lock taken, thus no
> further adjustments are necessary.
> 
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@xxxxxxxxx>

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks!



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.