Re: [PATCH 3/5] x86/hvm: Make HVM_PARAM_{STORE, CONSOLE}_EVTCHN read-only to the guest

From: Paul Durrant <Paul.Durrant@citrix.com>
To: Xen-devel <xen-devel@lists.xen.org>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Andrew Cooper <Andrew.Cooper3@citrix.com>,
	Julien Grall <julien.grall@arm.com>,
	Jan Beulich <JBeulich@suse.com>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH 3/5] x86/hvm: Make HVM_PARAM_{STORE, CONSOLE}_EVTCHN read-only to the guest
Date: Thu, 6 Sep 2018 09:16:30 +0000	[thread overview]
Message-ID: <c6b752b2b59f4f1293eb8d9f27a420eb@AMSPEX02CL03.citrite.net> (raw)
In-Reply-To: <1536171124-27053-4-git-send-email-andrew.cooper3@citrix.com>

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: 05 September 2018 19:12
> To: Xen-devel <xen-devel@lists.xen.org>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Jan Beulich
> <JBeulich@suse.com>; Wei Liu <wei.liu2@citrix.com>; Roger Pau Monne
> <roger.pau@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>; Stefano
> Stabellini <sstabellini@kernel.org>; Julien Grall <julien.grall@arm.com>
> Subject: [PATCH 3/5] x86/hvm: Make
> HVM_PARAM_{STORE,CONSOLE}_EVTCHN read-only to the guest
> 
> These values are set by the toolstack for each create/restore operation, and
> bound by xen{store,console}d before the the guest starts running.
> 
> A guest has no reason to modify them at all, and the matching *_PFN
> parameters
> are already read-only.  Adjust the *_EVTCHN permissions to be consistent.

Unfortunately this patch will break the Windows PV driver function here:

http://xenbits.xen.org/gitweb/?p=pvdrivers/win/xenbus.git;a=blob;f=src/xenbus/evtchn.c;hb=HEAD#l1037

Unfortunately the values really do change across a reset. It would be possible to use volatile (disappear on reboot) registry keys to store the updated values instead but I don't really see any harm in allowing the guest to update the values to be correct, unless we want to change Xen to do the job so the guest doesn't have to go through this dance.

  Paul

> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Paul Durrant <paul.durrant@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/x86/hvm/hvm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index d19ae35..408e695 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4079,8 +4079,6 @@ static int hvm_allow_set_param(struct domain *d,
>      case HVM_PARAM_VM86_TSS_SIZED:
>      case HVM_PARAM_ACPI_IOPORTS_LOCATION:
>      case HVM_PARAM_VM_GENERATION_ID_ADDR:
> -    case HVM_PARAM_STORE_EVTCHN:
> -    case HVM_PARAM_CONSOLE_EVTCHN:
>      case HVM_PARAM_X87_FIP_WIDTH:
>          break;
> 
> @@ -4090,6 +4088,7 @@ static int hvm_allow_set_param(struct domain *d,
>           * permissions in Xen, and therefore may not set by the domain.
>           */
>      case HVM_PARAM_STORE_PFN:
> +    case HVM_PARAM_STORE_EVTCHN:
>      case HVM_PARAM_PAE_ENABLED:
>      case HVM_PARAM_IOREQ_PFN:
>      case HVM_PARAM_BUFIOREQ_PFN:
> @@ -4101,6 +4100,7 @@ static int hvm_allow_set_param(struct domain *d,
>      case HVM_PARAM_ACPI_S_STATE:
>      case HVM_PARAM_VPT_ALIGN:
>      case HVM_PARAM_CONSOLE_PFN:
> +    case HVM_PARAM_CONSOLE_EVTCHN:
>      case HVM_PARAM_NESTEDHVM:
>      case HVM_PARAM_PAGING_RING_PFN:
>      case HVM_PARAM_MONITOR_RING_PFN:
> --
> 2.1.4

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel