From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Jan Beulich" <JBeulich@suse.com>
Subject: Re: [PATCH v2 2/2] x86/PCI: Intercept Dom0 MMCFG from
 dom0s in HVM containers
Date: Fri, 18 Dec 2015 03:28:21 -0700
Message-ID: <5673EDD502000078000C114D@prv-mh.provo.novell.com>
References: <1450383771-3636-1-git-send-email-boris.ostrovsky@oracle.com>
	<1450383771-3636-3-git-send-email-boris.ostrovsky@oracle.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1450383771-3636-3-git-send-email-boris.ostrovsky@oracle.com>
Content-Disposition: inline
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: andrew.cooper3@citrix.com, keir@xen.org, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

>>> On 17.12.15 at 21:22, <boris.ostrovsky@oracle.com> wrote:
> Commit 9256f66c1606 ("x86/PCI: intercept all PV Dom0 MMCFG writes")
> added intercepts for writes to RO MMCFG space from PV dom0.
> 
> Similar functionality is needed by dom0s in HVM containers (such as
> PVH and, in the future, HVMlite).
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

Looks okay leaving aside the question of r/o device handling. A few
minor remarks:

> @@ -463,4 +466,14 @@ void *
>  decode_register(
>      uint8_t modrm_reg, struct cpu_user_regs *regs, int highbyte_regs);
>  
> +/* Unhadleable read, write or instruction fetch */

Unhandleable

> +int
> +unhandleable_rwx(

x86emul_unhandleable_rwx() or some such, making it clear what
component it belongs to. I also doubt the x here really makes much
sense - the insn_fetch field can't usefully point to that function.

> +    enum x86_segment seg,
> +    unsigned long offset,
> +    void *p_data,
> +    unsigned int bytes,
> +    struct x86_emulate_ctxt *ctxt);
> +
> +
>  #endif /* __X86_EMULATE_H__ */

One blank line only please.

Jan