git: f4c6843ec2b9 - main - xen: use correct cache attributes for Xen specific memory regions

[John Baldwin]

On 8/12/21 8:53 AM, Roger Pau Monné wrote:
> On Thu, Aug 12, 2021 at 08:20:51AM -0700, John Baldwin wrote:
>> On 8/12/21 12:24 AM, Roger Pau Monné wrote:
>>> The branch main has been updated by royger:
>>>
>>> URL: https://cgit.FreeBSD.org/src/commit/?id=f4c6843ec2b9aa5eff475778fb000ed6278c5b77
>>>
>>> commit f4c6843ec2b9aa5eff475778fb000ed6278c5b77
>>> Author:     Roger Pau Monné <royger at FreeBSD.org>
>>> AuthorDate: 2021-04-09 09:31:44 +0000
>>> Commit:     Roger Pau Monné <royger at FreeBSD.org>
>>> CommitDate: 2021-08-12 07:18:32 +0000
>>>
>>>       xen: use correct cache attributes for Xen specific memory regions
>>>       bus_activate_resource maps memory regions as uncacheable on x86, which
>>>       is more strict than required for regions allocated using xenmem_alloc,
>>>       so don't rely on bus_activate_resource and instead map the region
>>>       using pmap_mapdev_attr and VM_MEMATTR_XEN as the cache attribute.
>>>       Sponsored by: Citrix Systems R&D
>>
>> It would probably be cleaner to use bus_map_resource() for this instead.  It
>> would mean you would have to use a structure that writes to as the argument
>> to bus_read/write_* instead of using the resource directly.
> 
> Those regions are usually handled to other subsystems of the kernel.
> They are mostly used to map memory from other domains and then perform
> IO on their behalf (like blkback and netback do), so it's not really
> possible to assert that all users of the regions would use
> bus_read/write_* to access them.
> 
> I could however switch to using bus_map_resource if I can pass the
> desired memory cache attribute and get a linear address back. It looks
> like resource_map_request parameter allows to select the cache
> attribute.

Yes, one of the use case for bus_map_resource is to permit passing a
non-default memory attribute.  (It also permits mapping sub-ranges, but
that isn't applicable in this case.)  The 'r_vaddr' in the resource_map
is the equivalent of rman_get_addr.  It'd be a little cleaner (and
perhaps friendly to future changes in this area) if your clients used
the 'r_vaddr' from the resource_map directly instead of using
rman_set_virtual() on the original resource.  (At some point I would like
to have each 'struct resource' keep a list of all the mappings created
from it and to forcefully invalidate/free any existing mappings that
still remain when a 'struct resource' is freed.  I imagine as part of
that I might end up with some assertions about the embedded resource
fields in the 'struct resource' being self-consistent).

-- 
John Baldwin