[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v4 09/27] memory: Prepare for different kinds of I
From: |
Auger Eric |
Subject: |
Re: [Qemu-devel] [RFC v4 09/27] memory: Prepare for different kinds of IOMMU MR notifiers |
Date: |
Tue, 28 May 2019 19:11:45 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
Hi Peter,
On 5/28/19 6:48 AM, Peter Xu wrote:
> On Mon, May 27, 2019 at 01:41:45PM +0200, Eric Auger wrote:
>
> [...]
>
>> @@ -3368,8 +3368,9 @@ static void vtd_address_space_unmap(VTDAddressSpace
>> *as, IOMMUNotifier *n)
>> {
>> IOMMUTLBEntry entry;
>> hwaddr size;
>> - hwaddr start = n->start;
>> - hwaddr end = n->end;
>> +
>
> (extra new line)
>
>> + hwaddr start = n->iotlb_notifier.start;
>> + hwaddr end = n->iotlb_notifier.end;
>> IntelIOMMUState *s = as->iommu_state;
>> DMAMap map;
>
> [...]
>
>> typedef void (*IOMMUNotify)(struct IOMMUNotifier *notifier,
>> IOMMUTLBEntry *data);
>>
>> -struct IOMMUNotifier {
>> +typedef struct IOMMUIOLTBNotifier {
>> IOMMUNotify notify;
>
> Hi, Eric,
>
> I wasn't following the thread much before so sorry to ask this if too
> late - have you thought about using the Notifier struct direct?
> Because then it'll (1) allow the user to register with both IOTLB |
> CONFIG flags in the same notifier while currently we'll need to
> register one for each (and this worries me a bit on when we grow the
> types of flags further then one register can have quite a few
> notifiers) (2) the notifier part can be shared by different events.
> Then when notify the (void *) data can be an union:
>
> struct IOMMUEvent {
> int event; // can be one of the notifier flags
> union {
> struct IOTLBEvent {
> ...
> };
> struct PASIDEvent {
> ...
> };
> }
> }
I am currently prototyping your suggestion. I think this would clarify
some parts of the code to see clearly the type of event that is
propagated. I will send a separate RFC for this change.
Thanks!
Eric
>
> Then the handler hook would be simple too:
>
> handler (data)
> {
> switch (data.event) {
> ...
> }
> }
>
> I would be fine with current patch if this series is close to be
> merged because even if we want that we can do that on top when we
> introduce even more notifiers, but just to ask loud first.
>
>> - IOMMUNotifierFlag notifier_flags;
>> /* Notify for address space range start <= addr <= end */
>> hwaddr start;
>> hwaddr end;
>> +} IOMMUIOLTBNotifier;
>> +
>> +struct IOMMUNotifier {
>> + IOMMUNotifierFlag notifier_flags;
>> + union {
>> + IOMMUIOLTBNotifier iotlb_notifier;
>> + };
>> int iommu_idx;
>> QLIST_ENTRY(IOMMUNotifier) node;
>> };
>> @@ -126,15 +132,18 @@ typedef struct IOMMUNotifier IOMMUNotifier;
>> /* RAM is a persistent kind memory */
>> #define RAM_PMEM (1 << 5)
>>
>> -static inline void iommu_notifier_init(IOMMUNotifier *n, IOMMUNotify fn,
>> - IOMMUNotifierFlag flags,
>> - hwaddr start, hwaddr end,
>> - int iommu_idx)
>> +static inline void iommu_iotlb_notifier_init(IOMMUNotifier *n, IOMMUNotify
>> fn,
>> + IOMMUNotifierFlag flags,
>> + hwaddr start, hwaddr end,
>> + int iommu_idx)
>> {
>> - n->notify = fn;
>> + assert(flags & IOMMU_NOTIFIER_IOTLB_MAP ||
>> + flags & IOMMU_NOTIFIER_IOTLB_UNMAP);
>
> Can use IOMMU_NOTIFIER_IOTLB_ALL directly?
>
>> + assert(start < end);
>> n->notifier_flags = flags;
>> - n->start = start;
>> - n->end = end;
>> + n->iotlb_notifier.notify = fn;
>> + n->iotlb_notifier.start = start;
>> + n->iotlb_notifier.end = end;
>> n->iommu_idx = iommu_idx;
>> }
>
> Otherwise the patch looks good to me.
>
> Regards,
>
- [Qemu-devel] [RFC v4 03/27] update-linux-headers: Add sve_context.h to asm-arm64, (continued)
- [Qemu-devel] [RFC v4 03/27] update-linux-headers: Add sve_context.h to asm-arm64, Eric Auger, 2019/05/27
- [Qemu-devel] [RFC v4 04/27] header update against 5.2.0-rc1 and IOMMU/VFIO nested stage APIs, Eric Auger, 2019/05/27
- [Qemu-devel] [RFC v4 05/27] memory: add IOMMU_ATTR_VFIO_NESTED IOMMU memory region attribute, Eric Auger, 2019/05/27
- [Qemu-devel] [RFC v4 06/27] memory: add IOMMU_ATTR_MSI_TRANSLATE IOMMU memory region attribute, Eric Auger, 2019/05/27
- [Qemu-devel] [RFC v4 07/27] hw/arm/smmuv3: Advertise VFIO_NESTED and MSI_TRANSLATE attributes, Eric Auger, 2019/05/27
- [Qemu-devel] [RFC v4 08/27] hw/vfio/common: Force nested if iommu requires it, Eric Auger, 2019/05/27
- [Qemu-devel] [RFC v4 09/27] memory: Prepare for different kinds of IOMMU MR notifiers, Eric Auger, 2019/05/27
- [Qemu-devel] [RFC v4 10/27] memory: Add IOMMUConfigNotifier, Eric Auger, 2019/05/27
- [Qemu-devel] [RFC v4 11/27] memory: Add arch_id and leaf fields in IOTLBEntry, Eric Auger, 2019/05/27
- [Qemu-devel] [RFC v4 12/27] hw/arm/smmuv3: Store the PASID table GPA in the translation config, Eric Auger, 2019/05/27
- [Qemu-devel] [RFC v4 13/27] hw/arm/smmuv3: Implement dummy replay, Eric Auger, 2019/05/27
- [Qemu-devel] [RFC v4 14/27] hw/arm/smmuv3: Fill the IOTLBEntry arch_id on NH_VA invalidation, Eric Auger, 2019/05/27
- [Qemu-devel] [RFC v4 15/27] hw/arm/smmuv3: Fill the IOTLBEntry leaf field on NH_VA invalidation, Eric Auger, 2019/05/27
- [Qemu-devel] [RFC v4 16/27] hw/arm/smmuv3: Notify on config changes, Eric Auger, 2019/05/27
- [Qemu-devel] [RFC v4 17/27] hw/vfio/common: Introduce vfio_alloc_guest_iommu helper, Eric Auger, 2019/05/27
- [Qemu-devel] [RFC v4 18/27] hw/vfio/common: Introduce hostwin_from_range helper, Eric Auger, 2019/05/27
- [Qemu-devel] [RFC v4 19/27] hw/vfio/common: Introduce helpers to DMA map/unmap a RAM section, Eric Auger, 2019/05/27