How to handle ARM SMMU translation faults correctly?

[sammy17]

gem5/src/dev/arm/smmu_v3_transl.cc

Line 232 in c890e6b

if (tr.fault != FAULT_NONE)

I noticed that in the current SMMU implementation, whenever there is a translation fault a panic is triggered, and the whole simulation is terminated.
However, according to the ARM SMMU specification, this translation should be aborted and the fault should be reported in the fault queue (if there are free slots).

How can I change the source code correctly so that translation requests that cause faults are aborted?

I tried replacing the panic call with the below lines, but seems like this also caused a random crash in the ethernet device which I did not observe before.
My guess is I am messing up the timing of the translation requests for some reason.

a.type = ACTION_TERMINATE;
yield(a);

Really appreciate any help on how to abort faulty requests correctly. @andysan @giactra

[giactra]

gem5/src/dev/arm/smmu_v3_transl.cc

Line 232 in c890e6b

if (tr.fault != FAULT_NONE)

I noticed that in the current SMMU implementation, whenever there is a translation fault a panic is triggered, and the whole simulation is terminated. However, according to the ARM SMMU specification, this translation should be aborted and the fault should be reported in the fault queue (if there are free slots).

How can I change the source code correctly so that translation requests that cause faults are aborted?

Yeah this is a missing feature of the SMMUv3 implementation in gem5. For sure you should use the sendEvent method to write into the EVENTQ before doing ACTION_TERMINATE.

I tried replacing the panic call with the below lines, but seems like this also caused a random crash in the ethernet device which I did not observe before. My guess is I am messing up the timing of the translation requests for some reason.

It could very well be the crash is due to a bug causing the translation failure and not the way you are handling the failure per se. I would first check the EVENTQ is written properly and the network driver is reading the status accordingly, and then start the more complex endeavour of chasing the motivation behind the translation fault...

a.type = ACTION_TERMINATE;
yield(a);

Really appreciate any help on how to abort faulty requests correctly. @andysan @giactra

[giactra]

Yeah the check in sendEvent is pretty much wrong

gem5/src/dev/arm/smmu_v3_transl.cc

Line 1331 in c890e6b

panic("eventq msi not enabled\n");

Support for MSIs is not reflected on the content of the EVENTQ_IRQ_CFG0 register per se (what we are checking in gem5), but rather on the SMMU_IDR0.MSI RO field. If you see the smmu_idr0 parameter in SMMUv3.py it is set to zero, which means the SMMU driver is perfectly aware MSIs are not supported and it is actually expecting a wired interrupt implementation.

To answer your question, the driver is supposed to register the physical address field in eventq_irq_cfg0 but it probably won't do it as we notified the driver the SMMU does not support MSIs.

I am not planning on implementing MSI support now, especially as it requires you to use a GICv3 ITS. I will instead post a PR which is adding wired interrupts for the eventq.

[sammy17]

Thanks @giactra !
Let me know when you have the PR.

In the meantime, I was able to handle the fault without crashing the simulation by calling the following handleFault function I defined. But after a while, the transmitting process of the ethernet device hangs, not sure why (maybe since I am not reporting the event to the event queue).
For now, this setup is fine for my current experiments so I will be using this until your PR is available.

void
SMMUTranslationProcess::handleFault(Yield &yield)
{
    SMMUAction a;
    a.type = ACTION_TERMINATE;
    a.pkt = NULL;
    a.ifc = &ifc;
    a.delay = 0;
    yield(a);
}

Thanks again, looking forward to your PR for getting things working correctly.

[giactra]

By simply terminating you are not returning anything to the requesting device (the NIC). So at a certain point you will have a set of in flight packets accumulating and the NIC will do no advancement in terms of memory transfer and it will just stall.

[sammy17]

I see, how to send a correct response to the device? And what is the response that should be sent in this case?
I tried defining a new SMMU action called ACTION_ABORT and tried sending a response to the device as below on translation fault.

void
SMMUTranslationProcess::handleFault(Yield &yield)
{
    smmu.scheduleDeviceRetries();

    SMMUAction a;
    a.type = ACTION_ABORT;
    a.pkt = request.pkt;
    a.ifc = &ifc;
    a.delay = 0;
    a.pkt->setHtmTransactionFailedInCache(a.pkt->getHtmTransactionFailedInCacheRC());
    yield(a);
}

Than at

gem5/src/dev/arm/smmu_v3.cc

Line 286 in 650b921

SMMUv3::runProcessTiming(SMMUProcess *proc, PacketPtr pkt)

I defined the action as below:

        case ACTION_ABORT:
            assert(action.ifc);
            action.ifc->schedTimingResp(action.pkt);
            delete proc;
            DPRINTF(SMMUv3, "ACTION_ABORT in timing mode\n");
            break;

However, I am getting an error with a.pkt->getHtmTransactionFailedInCacheRC() call.
Could you please let me know if the way I am trying to send the response to the device is correct?

[giactra]

getHtmTransactionFailedInCacheRC is an HTM method (hardware transactional memory) so you shouldn't be using it.
I am finishing up my PR and I hope to upstream it soon, but FYI this is more or less what I have been doing:

ifc.xlateSlotsRemaining++;
smmu.scheduleDeviceRetries();
                         
if (smmu.system.isAtomicMode()) {
    request.pkt->makeAtomicResponse();
} else if (smmu.system.isTimingMode()) {
    request.pkt->makeTimingResponse();
} else {                 
    panic("Not in atomic or timing mode");
}                        

// This is the key part
request.pkt->setBadAddress();
                         
SMMUAction a;            
// Send the bad address response to the client device
a.type = ACTION_SEND_RESP;
a.pkt = request.pkt; 
a.ifc = &ifc;            
a.delay = 0;             
yield(a); 

As you can see I am just returning a response packet as I would normally do (without accessing memory of course) but the difference is that I set the BAD_ADDRESS flag in the packet. I am not sure how the client device will respond to that (I can see the DmaDevice just emits a warning if an error packet is received), but I also believe signaling an error to the CPU (via the eventq) is crucial so that it can manage the SMMU to fix the translation

[giactra]

Hey @sammy17 please have a look at #920

[sammy17]

Thank you very much for the complete fix @giactra!
I will integrate this and test it, I will let you know how it goes.

[sammy17]

Hi @giactra I encountered an issue when I tried to generate the device-tree automatically with the PR.

ValueError: invalid literal for int() with base 0: 'eventq'

At:
  src/python/m5/util/fdthelper.py(59): <listcomp>
  src/python/m5/util/fdthelper.py(59): __init__
  src/dev/arm/SMMUv3.py(251): generateDeviceTree

Seems like it's an issue with parsing the device tree generation nodes.
I manually added the eventq interrupt and interrupt name to the device tree file now translation faults (and other faults) are handled correctly.
I also observed Linux kernel read these events correctly.
So everything works fine now except the automatic device-tree generation for eventq interrupt.
Thank you for the help!

[giactra]

Ops! #934

Nice!