From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel De Graaf Subject: Re: c/s 24425:053a44894279 (xsm: add checks on PCI configuration access) Date: Thu, 21 Jun 2012 11:14:03 -0400 Message-ID: <4FE33A3B.2090108@tycho.nsa.gov> References: <4FE33BD2020000780008B1B9@nat28.tlf.novell.com> <4FE32D56.6000809@tycho.nsa.gov> <4FE3561C020000780008B280@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4FE3561C020000780008B280@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Keir Fraser , xen-devel List-Id: xen-devel@lists.xenproject.org On 06/21/2012 11:13 AM, Jan Beulich wrote: >>>> On 21.06.12 at 16:19, Daniel De Graaf wrote: >> On 06/21/2012 09:20 AM, Jan Beulich wrote: >>> The mmconfig part of this is seriously broken: These operations, >>> even when carried out by Dom0, are MMIO accesses, and hence >>> are invisible to the hypervisor without extra handling. Putting >>> the checks into pci_mmcfg_{read,write}() has the effect of >>> potentially denying the _hypervisor_ access. So I think at least >>> that part needs to be reverted. >> >> I agree - the XSM checks are intended to be done only when the hypervisor >> is accessing on behalf of the domain, which looks to be covered by the >> traps part of the patch. These checks are currently intended to deny a >> domain with IS_PRIV but without full hardware access - in particular, >> without access to the PCI configuration MMIO area - from using legacy >> register access to reconfigure PCI devices. >> >> While it may be useful to extend this access check to include the PCI >> configuration MMIO pages, this would require emulating both reads and >> writes to any page that has entries that a particular domain does not >> have access to. The existing pciback/pcifront configuration access model >> already handles these issues without changes to the hypervisor. > > So do I read correctly that you agree to revert that part of said > c/s? > > Jan > Yes. -- Daniel De Graaf National Security Agency