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

Re: [PATCH 3/3] x86/ept: force WB cache attributes for grant and foreign maps


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Wed, 2 Jun 2021 11:36:58 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=pIQiLOdKv57R81Ktjktc4X+j3YRvkgqyEtOGYpMpneA=; b=VcREAnuKCFFVXPQneExNwmSG4USXFZv5baswR3sJsi4PqEqDhKOO7qn3YVuEbGKESsC+p/67ycfenz4OR6Ui6XC4ATWnxHC0J0FG7mVl6e46AVa/vevcZ0vPOpfJzIVuNu/i9Rt0zFVHZuDMsuJ9iYsBzxJGyNo+cxDOQmNKBdJvwlkcjI2YFBkyTW0XKSyHdlHR7QErTe6J/7jwWkaQf7UCW8oy4wd6NLDX7tXMAQt26q1b/rnlKed0LfwCU2a4gteaSA0q4vD2B22uFjpez7GYwSyy1PI55jzVD5k3jdDQ+sUxC6AUWQZLbqjLqgy8NdQ7f13P8CUKgx8w8U+ErA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Mlin6qe6porkE5UdemfXPXC5ZGBowzCafT+QUZIVEgfTBh5r/el32KBfeOYa8xFjYMsM+W+etE9N6nsEph7Yc6xHoNSU0fGiuIRDEIZRpwBeleZlmduK8NCOVrK3o4vmaGg7VJon6TFd8ohqGCYG5oIbTtjvx4ExKqzI7Wzf6FAH0PIcnRgwOYktkKIZ78E8JSy8RxKGqXBb2bn4Z5oOzXtSgnUdZv0E5yvhPGWaAvIC/Tkib9KZMNEuimEwmotDdRQ51f8vPDLmcKavmNFqHmxiXPmh03SK6PCZeT391XiA5g74cYVCE5vXMgOQpXqxPQ6LLMC3zdF3GuPXn/DWjQ==
  • Authentication-results: esa5.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Jun Nakajima <jun.nakajima@xxxxxxxxx>, Kevin Tian <kevin.tian@xxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, "George Dunlap" <george.dunlap@xxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 02 Jun 2021 09:37:24 +0000
  • Ironport-hdrordr: A9a23:hnIum6gP+F1h7uMyqxW5XlM5O3BQX+N13DAbv31ZSRFFG/FwyP rBoB1L73DJYWgqNE3I+erhBEGBKUmsk6KdxbNhQItKPTOWwldASbsC0WKM+UyEJ8STzJ846U 4kSdkDNDSSNykLsS+Z2njBLz9I+rDum8rE9ISurQYecegpUdAa0+4QMHfrLqQcfng+OXNWLu v62iIRzADQB0j/I/7LSUXsGIP41qv2fP2MW29JOzcXrC21yR+44r/zFBaVmj0EVSlU/Lsk+W /Z1yTk+6SKqZiAu1zh/l6Wy64TtMrqy9NFCsDJoNMSMC/QhgGhY5kkc6GevQoyvPqk5D8R4Z nxSi8bToFOAk7qDyWISUOH4Xim7N9u0Q6i9baguwqgnSSjLwhKTfao7OliA2jkA0lJhqA37E sE5RPBi3L7ZSmw1RgV3OK4Iy2CoHDE6kbKodRj+kC3brFuH4O5jbZvsX+9Q61wUB4T1ugcYa FT5ZbnlYlrmBWhHijkglU=
  • Ironport-sdr: /2p7dl3hRiFHA+XGdnu33ak8vPNR+CCRGFgFeNgSQWGmjWS/J6jErDQPQZx5WuO32gdWhicz9e MbfyubnOeMv1OX8sMV56jGaM9Q7ShHz/dWKoH3PIVBsCKltXrFnvz1uSg7B5rrH5KsfdSy8+pZ fJsHBFJsNkvCdYAwOYiVHnGla0hCRG6FBdDyDyj8bnKpak0Gk9SEV4qdL2Mzfsfmy73+1L7DdA 6nxBfhclmJTVTUz6S+r1rl7h2iVpTmFwtnW9gzS18nQmlY33lRQIIOHKHf+A5LktrNj4Gnzcfj N6E=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Mon, May 31, 2021 at 09:21:25AM +0200, Jan Beulich wrote:
> On 28.05.2021 19:39, Roger Pau Monne wrote:
> > --- a/xen/arch/x86/mm/p2m-ept.c
> > +++ b/xen/arch/x86/mm/p2m-ept.c
> > @@ -487,11 +487,12 @@ static int ept_invalidate_emt_range(struct p2m_domain 
> > *p2m,
> >  }
> >  
> >  int epte_get_entry_emt(struct domain *d, gfn_t gfn, mfn_t mfn,
> > -                       unsigned int order, bool *ipat, bool direct_mmio)
> > +                       unsigned int order, bool *ipat, p2m_type_t type)
> >  {
> >      int gmtrr_mtype, hmtrr_mtype;
> >      struct vcpu *v = current;
> >      unsigned long i;
> > +    bool direct_mmio = type == p2m_mmio_direct;
> 
> I don't think this variable is worthwhile to retain/introduce:
> 
> > @@ -535,9 +536,33 @@ int epte_get_entry_emt(struct domain *d, gfn_t gfn, 
> > mfn_t mfn,
> >          }
> >      }
> >  
> > -    if ( direct_mmio )
> 
> With this gone, there's exactly one further use left. Preferably
> with this adjustment (which I'd be fine to make while committing, as
> long as you and/or the maintainers agree)
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks. That's fine, I was about to drop it but didn't want to introduce any
more changes than necessary.

> 
> > +    switch ( type )
> > +    {
> > +    case p2m_mmio_direct:
> >          return MTRR_TYPE_UNCACHABLE;
> 
> As a largely unrelated note: We really want to find a way to return
> WC here for e.g. the frame buffer of graphics cards, the more that
> hvm_get_mem_pinned_cacheattr() gets invoked only below from here
> (unlike at initial introduction of the function, where it was called
> ahead of the direct_mmio check, but still after the mfn_valid(), so
> the results were inconsistent anyway). Perhaps we should obtain the
> host MTRR setting for the page (or range) in question.
> 
> As to hvm_get_mem_pinned_cacheattr(), XEN_DOMCTL_pin_mem_cacheattr
> is documented to be intended to be used on RAM only anyway ...

I also think we should make epte_get_entry_emt available to all p2m
code so it can partially replace the logic in p2m_type_to_flags to
account for cache attributes. I don't think there's much point in
keeping such different methods for accounting for cache attributes. I
know AMD lacks an ignore PAT equivalent, but there's no reason why p2m
cache attributes calculation should be done differently for AMD and
Intel AFAICT.

Roger.



 


Rackspace

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