| Message ID | 20240702055221.1337035-20-clement.mathieu--drif@eviden.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ATS support for VT-d | expand |
On 2024/7/2 13:52, CLEMENT MATHIEU--DRIF wrote: > From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com> > > IOMMU have to implement iommu_ats_request_translation to support ATS. > > Devices can use IOMMU_TLB_ENTRY_TRANSLATION_ERROR to check the tlb > entries returned by a translation request. > > Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com> > --- > include/exec/memory.h | 26 ++++++++++++++++++++++++++ > system/memory.c | 20 ++++++++++++++++++++ > 2 files changed, 46 insertions(+) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 003ee06610..48555c87c6 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -148,6 +148,10 @@ struct IOMMUTLBEntry { > uint32_t pasid; > }; > > +/* Check if an IOMMU TLB entry indicates a translation error */ > +#define IOMMU_TLB_ENTRY_TRANSLATION_ERROR(entry) ((((entry)->perm) & IOMMU_RW) \ > + == IOMMU_NONE) > + > /* > * Bitmap for different IOMMUNotifier capabilities. Each notifier can > * register with one or multiple IOMMU Notifier capability bit(s). > @@ -571,6 +575,20 @@ struct IOMMUMemoryRegionClass { > int (*iommu_set_iova_ranges)(IOMMUMemoryRegion *iommu, > GList *iova_ranges, > Error **errp); > + > + /** > + * @iommu_ats_request_translation: > + * This method must be implemented if the IOMMU has ATS enabled > + * > + * @see pci_ats_request_translation_pasid > + */ > + ssize_t (*iommu_ats_request_translation)(IOMMUMemoryRegion *iommu, > + bool priv_req, bool exec_req, > + hwaddr addr, size_t length, > + bool no_write, > + IOMMUTLBEntry *result, > + size_t result_length, > + uint32_t *err_count); > }; > I'm not quite understanding why the existing translate() does not work. Could you elaborate? > typedef struct RamDiscardListener RamDiscardListener; > @@ -1926,6 +1944,14 @@ void memory_region_iommu_replay(IOMMUMemoryRegion *iommu_mr, IOMMUNotifier *n); > void memory_region_unregister_iommu_notifier(MemoryRegion *mr, > IOMMUNotifier *n); > > +ssize_t memory_region_iommu_ats_request_translation(IOMMUMemoryRegion *iommu_mr, > + bool priv_req, bool exec_req, > + hwaddr addr, size_t length, > + bool no_write, > + IOMMUTLBEntry *result, > + size_t result_length, > + uint32_t *err_count); > + > /** > * memory_region_iommu_get_attr: return an IOMMU attr if get_attr() is > * defined on the IOMMU. > diff --git a/system/memory.c b/system/memory.c > index 74cd73ebc7..8268df7bf5 100644 > --- a/system/memory.c > +++ b/system/memory.c > @@ -2005,6 +2005,26 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr, > memory_region_update_iommu_notify_flags(iommu_mr, NULL); > } > > +ssize_t memory_region_iommu_ats_request_translation(IOMMUMemoryRegion *iommu_mr, > + bool priv_req, > + bool exec_req, > + hwaddr addr, size_t length, > + bool no_write, > + IOMMUTLBEntry *result, > + size_t result_length, > + uint32_t *err_count) > +{ > + IOMMUMemoryRegionClass *imrc = memory_region_get_iommu_class_nocheck(iommu_mr); > + > + if (!imrc->iommu_ats_request_translation) { > + return -ENODEV; > + } > + > + return imrc->iommu_ats_request_translation(iommu_mr, priv_req, exec_req, > + addr, length, no_write, result, > + result_length, err_count); > +} > + > void memory_region_notify_iommu_one(IOMMUNotifier *notifier, > IOMMUTLBEvent *event) > {
On 03/07/2024 14:14, Yi Liu wrote: > Caution: External email. Do not open attachments or click links, > unless this email comes from a known sender and you know the content > is safe. > > > On 2024/7/2 13:52, CLEMENT MATHIEU--DRIF wrote: >> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com> >> >> IOMMU have to implement iommu_ats_request_translation to support ATS. >> >> Devices can use IOMMU_TLB_ENTRY_TRANSLATION_ERROR to check the tlb >> entries returned by a translation request. >> >> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com> >> --- >> include/exec/memory.h | 26 ++++++++++++++++++++++++++ >> system/memory.c | 20 ++++++++++++++++++++ >> 2 files changed, 46 insertions(+) >> >> diff --git a/include/exec/memory.h b/include/exec/memory.h >> index 003ee06610..48555c87c6 100644 >> --- a/include/exec/memory.h >> +++ b/include/exec/memory.h >> @@ -148,6 +148,10 @@ struct IOMMUTLBEntry { >> uint32_t pasid; >> }; >> >> +/* Check if an IOMMU TLB entry indicates a translation error */ >> +#define IOMMU_TLB_ENTRY_TRANSLATION_ERROR(entry) ((((entry)->perm) & >> IOMMU_RW) \ >> + == IOMMU_NONE) >> + >> /* >> * Bitmap for different IOMMUNotifier capabilities. Each notifier can >> * register with one or multiple IOMMU Notifier capability bit(s). >> @@ -571,6 +575,20 @@ struct IOMMUMemoryRegionClass { >> int (*iommu_set_iova_ranges)(IOMMUMemoryRegion *iommu, >> GList *iova_ranges, >> Error **errp); >> + >> + /** >> + * @iommu_ats_request_translation: >> + * This method must be implemented if the IOMMU has ATS enabled >> + * >> + * @see pci_ats_request_translation_pasid >> + */ >> + ssize_t (*iommu_ats_request_translation)(IOMMUMemoryRegion *iommu, >> + bool priv_req, bool >> exec_req, >> + hwaddr addr, size_t >> length, >> + bool no_write, >> + IOMMUTLBEntry *result, >> + size_t result_length, >> + uint32_t *err_count); >> }; >> > > I'm not quite understanding why the existing translate() does not work. > Could you elaborate? We need more parameters than what the existing translation function has. This one is designed to get translations for a range instead of just a single address. The main purpose is to expose an API that has the same parameters as a PCIe translation request message and to give all the information the IOMMU needs to process the request. > >> typedef struct RamDiscardListener RamDiscardListener; >> @@ -1926,6 +1944,14 @@ void >> memory_region_iommu_replay(IOMMUMemoryRegion *iommu_mr, IOMMUNotifier >> *n); >> void memory_region_unregister_iommu_notifier(MemoryRegion *mr, >> IOMMUNotifier *n); >> >> +ssize_t >> memory_region_iommu_ats_request_translation(IOMMUMemoryRegion *iommu_mr, >> + bool priv_req, bool >> exec_req, >> + hwaddr addr, size_t >> length, >> + bool no_write, >> + IOMMUTLBEntry *result, >> + size_t result_length, >> + uint32_t *err_count); >> + >> /** >> * memory_region_iommu_get_attr: return an IOMMU attr if get_attr() is >> * defined on the IOMMU. >> diff --git a/system/memory.c b/system/memory.c >> index 74cd73ebc7..8268df7bf5 100644 >> --- a/system/memory.c >> +++ b/system/memory.c >> @@ -2005,6 +2005,26 @@ void >> memory_region_unregister_iommu_notifier(MemoryRegion *mr, >> memory_region_update_iommu_notify_flags(iommu_mr, NULL); >> } >> >> +ssize_t >> memory_region_iommu_ats_request_translation(IOMMUMemoryRegion *iommu_mr, >> + bool priv_req, >> + bool exec_req, >> + hwaddr addr, >> size_t length, >> + bool no_write, >> + IOMMUTLBEntry *result, >> + size_t >> result_length, >> + uint32_t >> *err_count) >> +{ >> + IOMMUMemoryRegionClass *imrc = >> memory_region_get_iommu_class_nocheck(iommu_mr); >> + >> + if (!imrc->iommu_ats_request_translation) { >> + return -ENODEV; >> + } >> + >> + return imrc->iommu_ats_request_translation(iommu_mr, priv_req, >> exec_req, >> + addr, length, >> no_write, result, >> + result_length, >> err_count); >> +} >> + >> void memory_region_notify_iommu_one(IOMMUNotifier *notifier, >> IOMMUTLBEvent *event) >> { > > -- > Regards, > Yi Liu
On 2024/7/4 12:30, CLEMENT MATHIEU--DRIF wrote: > > On 03/07/2024 14:14, Yi Liu wrote: >> Caution: External email. Do not open attachments or click links, >> unless this email comes from a known sender and you know the content >> is safe. >> >> >> On 2024/7/2 13:52, CLEMENT MATHIEU--DRIF wrote: >>> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com> >>> >>> IOMMU have to implement iommu_ats_request_translation to support ATS. >>> >>> Devices can use IOMMU_TLB_ENTRY_TRANSLATION_ERROR to check the tlb >>> entries returned by a translation request. >>> >>> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com> >>> --- >>> include/exec/memory.h | 26 ++++++++++++++++++++++++++ >>> system/memory.c | 20 ++++++++++++++++++++ >>> 2 files changed, 46 insertions(+) >>> >>> diff --git a/include/exec/memory.h b/include/exec/memory.h >>> index 003ee06610..48555c87c6 100644 >>> --- a/include/exec/memory.h >>> +++ b/include/exec/memory.h >>> @@ -148,6 +148,10 @@ struct IOMMUTLBEntry { >>> uint32_t pasid; >>> }; >>> >>> +/* Check if an IOMMU TLB entry indicates a translation error */ >>> +#define IOMMU_TLB_ENTRY_TRANSLATION_ERROR(entry) ((((entry)->perm) & >>> IOMMU_RW) \ >>> + == IOMMU_NONE) >>> + >>> /* >>> * Bitmap for different IOMMUNotifier capabilities. Each notifier can >>> * register with one or multiple IOMMU Notifier capability bit(s). >>> @@ -571,6 +575,20 @@ struct IOMMUMemoryRegionClass { >>> int (*iommu_set_iova_ranges)(IOMMUMemoryRegion *iommu, >>> GList *iova_ranges, >>> Error **errp); >>> + >>> + /** >>> + * @iommu_ats_request_translation: >>> + * This method must be implemented if the IOMMU has ATS enabled >>> + * >>> + * @see pci_ats_request_translation_pasid >>> + */ >>> + ssize_t (*iommu_ats_request_translation)(IOMMUMemoryRegion *iommu, >>> + bool priv_req, bool >>> exec_req, >>> + hwaddr addr, size_t >>> length, >>> + bool no_write, >>> + IOMMUTLBEntry *result, >>> + size_t result_length, >>> + uint32_t *err_count); >>> }; >>> >> >> I'm not quite understanding why the existing translate() does not work. >> Could you elaborate? > We need more parameters than what the existing translation function has. > This one is designed to get translations for a range instead of just a > single address. > The main purpose is to expose an API that has the same parameters as a > PCIe translation request message > and to give all the information the IOMMU needs to process the request. ok. Please make the reason clear in commit message as well. Let's see if any other opinion on it. >> >>> typedef struct RamDiscardListener RamDiscardListener; >>> @@ -1926,6 +1944,14 @@ void >>> memory_region_iommu_replay(IOMMUMemoryRegion *iommu_mr, IOMMUNotifier >>> *n); >>> void memory_region_unregister_iommu_notifier(MemoryRegion *mr, >>> IOMMUNotifier *n); >>> >>> +ssize_t >>> memory_region_iommu_ats_request_translation(IOMMUMemoryRegion *iommu_mr, >>> + bool priv_req, bool >>> exec_req, >>> + hwaddr addr, size_t >>> length, >>> + bool no_write, >>> + IOMMUTLBEntry *result, >>> + size_t result_length, >>> + uint32_t *err_count); >>> + >>> /** >>> * memory_region_iommu_get_attr: return an IOMMU attr if get_attr() is >>> * defined on the IOMMU. >>> diff --git a/system/memory.c b/system/memory.c >>> index 74cd73ebc7..8268df7bf5 100644 >>> --- a/system/memory.c >>> +++ b/system/memory.c >>> @@ -2005,6 +2005,26 @@ void >>> memory_region_unregister_iommu_notifier(MemoryRegion *mr, >>> memory_region_update_iommu_notify_flags(iommu_mr, NULL); >>> } >>> >>> +ssize_t >>> memory_region_iommu_ats_request_translation(IOMMUMemoryRegion *iommu_mr, >>> + bool priv_req, >>> + bool exec_req, >>> + hwaddr addr, >>> size_t length, >>> + bool no_write, >>> + IOMMUTLBEntry *result, >>> + size_t >>> result_length, >>> + uint32_t >>> *err_count) >>> +{ >>> + IOMMUMemoryRegionClass *imrc = >>> memory_region_get_iommu_class_nocheck(iommu_mr); >>> + >>> + if (!imrc->iommu_ats_request_translation) { >>> + return -ENODEV; >>> + } >>> + >>> + return imrc->iommu_ats_request_translation(iommu_mr, priv_req, >>> exec_req, >>> + addr, length, >>> no_write, result, >>> + result_length, >>> err_count); >>> +} >>> + >>> void memory_region_notify_iommu_one(IOMMUNotifier *notifier, >>> IOMMUTLBEvent *event) >>> { >> >> -- >> Regards, >> Yi Liu
diff --git a/include/exec/memory.h b/include/exec/memory.h index 003ee06610..48555c87c6 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -148,6 +148,10 @@ struct IOMMUTLBEntry { uint32_t pasid; }; +/* Check if an IOMMU TLB entry indicates a translation error */ +#define IOMMU_TLB_ENTRY_TRANSLATION_ERROR(entry) ((((entry)->perm) & IOMMU_RW) \ + == IOMMU_NONE) + /* * Bitmap for different IOMMUNotifier capabilities. Each notifier can * register with one or multiple IOMMU Notifier capability bit(s). @@ -571,6 +575,20 @@ struct IOMMUMemoryRegionClass { int (*iommu_set_iova_ranges)(IOMMUMemoryRegion *iommu, GList *iova_ranges, Error **errp); + + /** + * @iommu_ats_request_translation: + * This method must be implemented if the IOMMU has ATS enabled + * + * @see pci_ats_request_translation_pasid + */ + ssize_t (*iommu_ats_request_translation)(IOMMUMemoryRegion *iommu, + bool priv_req, bool exec_req, + hwaddr addr, size_t length, + bool no_write, + IOMMUTLBEntry *result, + size_t result_length, + uint32_t *err_count); }; typedef struct RamDiscardListener RamDiscardListener; @@ -1926,6 +1944,14 @@ void memory_region_iommu_replay(IOMMUMemoryRegion *iommu_mr, IOMMUNotifier *n); void memory_region_unregister_iommu_notifier(MemoryRegion *mr, IOMMUNotifier *n); +ssize_t memory_region_iommu_ats_request_translation(IOMMUMemoryRegion *iommu_mr, + bool priv_req, bool exec_req, + hwaddr addr, size_t length, + bool no_write, + IOMMUTLBEntry *result, + size_t result_length, + uint32_t *err_count); + /** * memory_region_iommu_get_attr: return an IOMMU attr if get_attr() is * defined on the IOMMU. diff --git a/system/memory.c b/system/memory.c index 74cd73ebc7..8268df7bf5 100644 --- a/system/memory.c +++ b/system/memory.c @@ -2005,6 +2005,26 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr, memory_region_update_iommu_notify_flags(iommu_mr, NULL); } +ssize_t memory_region_iommu_ats_request_translation(IOMMUMemoryRegion *iommu_mr, + bool priv_req, + bool exec_req, + hwaddr addr, size_t length, + bool no_write, + IOMMUTLBEntry *result, + size_t result_length, + uint32_t *err_count) +{ + IOMMUMemoryRegionClass *imrc = memory_region_get_iommu_class_nocheck(iommu_mr); + + if (!imrc->iommu_ats_request_translation) { + return -ENODEV; + } + + return imrc->iommu_ats_request_translation(iommu_mr, priv_req, exec_req, + addr, length, no_write, result, + result_length, err_count); +} + void memory_region_notify_iommu_one(IOMMUNotifier *notifier, IOMMUTLBEvent *event) {