Re: [PATCH 09/37] iommu/fault: Let handler return a fault response
On 20/02/18 23:19, Jacob Pan wrote: > On Mon, 12 Feb 2018 18:33:24 + > Jean-Philippe Brucker wrote: > >> >> +/** >> + * enum page_response_code - Return status of fault handlers, >> telling the IOMMU >> + * driver how to proceed with the fault. >> + * >> + * @IOMMU_FAULT_STATUS_HANDLED: Stop processing the fault, and do >> not send a >> + * reply to the device. >> + * @IOMMU_FAULT_STATUS_CONTINUE: Fault was not handled. Call the >> next handler, >> + * or terminate. >> + * @IOMMU_FAULT_STATUS_SUCCESS: Fault has been handled and the page >> tables >> + * populated, retry the access. This is "Success" in PCI PRI. >> + * @IOMMU_FAULT_STATUS_FAILURE: General error. Drop all subsequent >> faults from >> + * this device if possible. This is "Response Failure" in PCI >> PRI. >> + * @IOMMU_FAULT_STATUS_INVALID: Could not handle this fault, don't >> retry the >> + * access. This is "Invalid Request" in PCI PRI. >> + */ >> +enum page_response_code { >> +IOMMU_PAGE_RESP_HANDLED = 0, >> +IOMMU_PAGE_RESP_CONTINUE, >> +IOMMU_PAGE_RESP_SUCCESS, >> +IOMMU_PAGE_RESP_INVALID, >> +IOMMU_PAGE_RESP_FAILURE, >> +}; > it seems to me two things are mixed here: > 1. driver handler response status (HANDLED, CONTINUE) > 2. PCI standard page response code (the rest) > Can we leave them separate? then we don't have to convert this enum > to/from PCI ATS page response code. Except when the producer is a platform device instead of PCI :) But I get your point. I liked combining them into one enum because it may simplify some device drivers. I can separate HANDLED/CONTINUE and have drivers always call page_response(). Thanks, Jean ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 09/37] iommu/fault: Let handler return a fault response
On Mon, 12 Feb 2018 18:33:24 + Jean-Philippe Brucker wrote: > > +/** > + * enum page_response_code - Return status of fault handlers, > telling the IOMMU > + * driver how to proceed with the fault. > + * > + * @IOMMU_FAULT_STATUS_HANDLED: Stop processing the fault, and do > not send a > + * reply to the device. > + * @IOMMU_FAULT_STATUS_CONTINUE: Fault was not handled. Call the > next handler, > + * or terminate. > + * @IOMMU_FAULT_STATUS_SUCCESS: Fault has been handled and the page > tables > + * populated, retry the access. This is "Success" in PCI PRI. > + * @IOMMU_FAULT_STATUS_FAILURE: General error. Drop all subsequent > faults from > + * this device if possible. This is "Response Failure" in PCI > PRI. > + * @IOMMU_FAULT_STATUS_INVALID: Could not handle this fault, don't > retry the > + * access. This is "Invalid Request" in PCI PRI. > + */ > +enum page_response_code { > + IOMMU_PAGE_RESP_HANDLED = 0, > + IOMMU_PAGE_RESP_CONTINUE, > + IOMMU_PAGE_RESP_SUCCESS, > + IOMMU_PAGE_RESP_INVALID, > + IOMMU_PAGE_RESP_FAILURE, > +}; it seems to me two things are mixed here: 1. driver handler response status (HANDLED, CONTINUE) 2. PCI standard page response code (the rest) Can we leave them separate? then we don't have to convert this enum to/from PCI ATS page response code. > + > /** > * Generic page response information based on PCI ATS and PASID spec. > * @addr: servicing page address > @@ -202,12 +225,7 @@ enum page_response_type { > struct page_response_msg { > u64 addr; > u32 pasid; > - u32 resp_code:4; > -#define IOMMU_PAGE_RESP_SUCCESS 0 > -#define IOMMU_PAGE_RESP_INVALID 1 > -#define IOMMU_PAGE_RESP_HANDLED 2 > -#define IOMMU_PAGE_RESP_FAILURE 0xF > - [Jacob Pan] ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu