On Mon, Jul 24, 2023 at 03:31:56PM +0200, Jan Beulich wrote:
> On 24.07.2023 15:16, Roger Pau Monné wrote:
> > On Mon, Jul 24, 2023 at 12:43:26PM +0200, Jan Beulich wrote:
> >> On 20.07.2023 02:32, Volodymyr Babchuk wrote:
> >>> @@ -52,8 +66,8 @@ static int cf_check map_range(
> >>> * - {un}map_mmio_regions doesn't support preemption.
> >>> */
> >>>
> >>> - rc = map->map ? map_mmio_regions(map->d, _gfn(s), size, _mfn(s))
> >>> - : unmap_mmio_regions(map->d, _gfn(s), size,
> >>> _mfn(s));
> >>> + rc = map->map ? map_mmio_regions(map->d, start_gfn, size,
> >>> _mfn(s))
> >>> + : unmap_mmio_regions(map->d, start_gfn, size,
> >>> _mfn(s));
> >>
> >> Aiui this is the first direct exposure of these functions to DomU-s;
> >
> > I guess it depends on how direct you consider exposure from
> > XEN_DOMCTL_memory_mapping hypercall, as that's what gets called by
> > QEMU also in order to set up BAR mappings.
>
> Fair point - it is one of the few domctls not covered by XSA-77.
>
> >> so far all calls were Xen-internal or from a domctl. There are a
> >> couple of Arm TODOs listed in the comment ahead, but I'm not sure
> >> that's all what is lacking here, and it's unclear whether this can
> >> sensibly be left as a follow-on activity (at the very least known
> >> open issues need mentioning as TODOs).
> >>
> >> For example the x86 function truncates an unsigned long local
> >> variable to (signed) int in its main return statement. This may for
> >> the moment still be only a theoretical issue, but will need dealing
> >> with sooner or later, I think.
> >
> > One bit that we need to add is the iomem_access_permitted() plus the
> > xsm_iomem_mapping() checks to map_range().
>
> The former would just be reassurance, wouldn't it? Assigning a PCI
> device surely implies granting access to all its BARs (minus the
> MSI-X page(s), if any).
Indeed. But for consistency we need to match the same checks that are
done in XEN_DOMCTL_memory_mapping.
> The latter would of course be more
> "interesting", as XSM could in principle interject.
Yes, we need both. In fact I'm just writing a patch to add them
straight away.
Thanks, Roger.
