On Wed, Jun 26, 2024 at 2:15 PM cmd <clement.mathieudrif....@gmail.com>
wrote:
> Hi,
>
> On 25/06/2024 13:28, Yuke Peng wrote:
> > According to Intel VT-d specification 5.1.4, CFI must be blocked when
> > Extended Interrupt Mode is enabled or Compatibility format interrupts
> > are disabled.
> >
> > Signed-off-by: Yuke Peng <pykfi...@gmail.com>
> > ---
> > hw/i386/intel_iommu.c | 28 ++++++++++++++++++++++++++++
> > hw/i386/trace-events | 1 +
> > include/hw/i386/intel_iommu.h | 1 +
> > 3 files changed, 30 insertions(+)
> >
> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > index 5085a6fee3..dfa2f979e7 100644
> > --- a/hw/i386/intel_iommu.c
> > +++ b/hw/i386/intel_iommu.c
> > @@ -2410,6 +2410,22 @@ static void vtd_handle_gcmd_ire(IntelIOMMUState
> *s, bool en)
> > }
> > }
> >
> > +/* Handle Compatibility Format Interrupts Enable/Disable */
> > +static void vtd_handle_gcmd_cfi(IntelIOMMUState *s, bool en)
> > +{
> > + trace_vtd_cfi_enable(en);
> > +
> > + if (en) {
> > + s->cfi_enabled = true;
> > + /* Ok - report back to driver */
> > + vtd_set_clear_mask_long(s, DMAR_GSTS_REG, 0, VTD_GSTS_CFIS);
> > + } else {
> > + s->cfi_enabled = false;
> > + /* Ok - report back to driver */
> > + vtd_set_clear_mask_long(s, DMAR_GSTS_REG, VTD_GSTS_CFIS, 0);
> > + }
> You have the same comment in both branches of the if statement, couldn't
> we put it between 'trace' and 'if'?
>
The reason I put the comment within the branches is that the function
'vtd_handle_gcmd_cfi' is similar to the function 'vtd_handle_gcmd_ire'
and 'vtd_handle_gcmd_te'. So for consistency, I also put comments
within the branches instead of in front of 'if'. Besides, I think the
comments
only explain the statement 'vtd_set_clear_mask_long'.
The 'vtd_handle_gcmd_ire' function:
> /* Handle Interrupt Remap Enable/Disable */
> static void vtd_handle_gcmd_ire(IntelIOMMUState *s, bool en)
> {
> trace_vtd_ir_enable(en);
>
> if (en) {
> s->intr_enabled = true;
> /* Ok - report back to driver */
> vtd_set_clear_mask_long(s, DMAR_GSTS_REG, 0, VTD_GSTS_IRES);
> } else {
> s->intr_enabled = false;
> /* Ok - report back to driver */
> vtd_set_clear_mask_long(s, DMAR_GSTS_REG, VTD_GSTS_IRES, 0);
> }
> }
