Re: [PATCH v8 03/13] vpci: restrict unhandled read/write operations for guests

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Jul 20, 2023 at 12:32:31AM +0000, Volodymyr Babchuk wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> 
> A guest would be able to read and write those registers which are not
> emulated and have no respective vPCI handlers, so it will be possible
> for it to access the hardware directly.
> In order to prevent a guest from reads and writes from/to the unhandled
                                                            ^ extra 'the'
> registers make sure only hardware domain can access the hardware directly
> and restrict guests from doing so.
> 
> Suggested-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>

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

With the stray change below removed.

> 
> ---
> Since v6:
> - do not use is_hwdom parameter for vpci_{read|write}_hw and use
>   current->domain internally
> - update commit message
> New in v6
> ---
>  xen/drivers/vpci/vpci.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
> index f22cbf2112..a6d2cf8660 100644
> --- a/xen/drivers/vpci/vpci.c
> +++ b/xen/drivers/vpci/vpci.c
> @@ -233,6 +233,10 @@ static uint32_t vpci_read_hw(pci_sbdf_t sbdf, unsigned 
> int reg,
>  {
>      uint32_t data;
>  
> +    /* Guest domains are not allowed to read real hardware. */
> +    if ( !is_hardware_domain(current->domain) )
> +        return ~(uint32_t)0;
> +
>      switch ( size )
>      {
>      case 4:
> @@ -273,9 +277,13 @@ static uint32_t vpci_read_hw(pci_sbdf_t sbdf, unsigned 
> int reg,
>      return data;
>  }
>  
> -static void vpci_write_hw(pci_sbdf_t sbdf, unsigned int reg, unsigned int 
> size,
> -                          uint32_t data)
> +static void vpci_write_hw(pci_sbdf_t sbdf, unsigned int reg,
> +                          unsigned int size, uint32_t data)

Unrelated change?

Thanks, Roger.