On Tue, Mar 14, 2023 at 08:56:30PM +0000, Volodymyr Babchuk wrote:
> From: Oleksandr Andrushchenko <[email protected]>
>
> 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
> registers make sure only hardware domain can access the hardware directly
> and restrict guests from doing so.
>
> Suggested-by: Roger Pau Monné <[email protected]>
> Signed-off-by: Oleksandr Andrushchenko <[email protected]>
>
> ---
>
> v3:
> - No changes
>
> Older comments from another series:
>
> Since v6:
> - do not use is_hwdom parameter for vpci_{read|write}_hw and use
> current->domain internally
> - update commit message
> New in v6
> Moved into another series
> ---
> 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 5232f9605b..199ff55672 100644
> --- a/xen/drivers/vpci/vpci.c
> +++ b/xen/drivers/vpci/vpci.c
> @@ -220,6 +220,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:
> @@ -260,9 +264,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? The parameter list doesn't go over 80 characters so
this rearranging is not necessary, and in any case should be done in a
separate commit or at least mentioned in the commit log.
> {
> + /* Guest domains are not allowed to write real hardware. */
I would maybe write this as:
"Unprivileged domain are not allowed unhandled accesses to the config
space."
But that's mostly a nit, and would also apply to the comment in
vpci_read_hw().
Thanks, Roger.
