In D53232#1216532, @jrtc27 wrote:

This will not fix that bug. fence is fence iorw, iorw, so all you are doing is relaxing some ordering requirements.

This will not fix that bug. fence is fence iorw, iorw, so all you are doing is relaxing some ordering requirements.

@jrtc27 I've update the revision, PTAL

In D53232#1216063, @jrtc27 wrote:

Do we actually have a proper definition of what *our* memory barrier APIs are meant to mean? What the ISA manual says is a sensible mapping isn't necessarily what we want; note that the table you're referencing is for "Linux memory primitives", and we are not Linux, nor do we follow its memory model.

Do we actually have a proper definition of what *our* memory barrier APIs are meant to mean? What the ISA manual says is a sensible mapping isn't necessarily what we want; note that the table you're referencing is for "Linux memory primitives", and we are not Linux, nor do we follow its memory model.

Shall I send a new patch? No comments on this one.

FYI, it's best practice to generate diffs with full context (-U999999 or whatever).

• Refactor aplic code into common init and aplic/imsic specific code.
• Add support for IMSIC.

In D43452#1055467, @br wrote:

Hi Himanshu what is status of this? could you regenerate for the latest HEAD ?

Hi Himanshu what is status of this? could you regenerate for the latest HEAD ?

Reworked APLIC_IDC_REG as asked

Sorry for the mess up in versions in comments. Last patch was v9, as per my local branches. This one is v10.

Changes in v10:

• Save hart indices from FDT and use them to create target value

LGTM!

ping.

After Convert local interrupt controller to a newbus PIC, commit rebase is required.

Changes in v7:

• Keep per-cpu IDC offsets after reading interrupts-extended property
• Accessing IDC is via the stored per-CPU IDC offsets
• Remove cleanup after failure in *_attach function (like other drivers)
• Added aplic.c in files.riscv alphabetically

In D43293#993161, @himanshu_thechauhan.dev wrote:

In D43293#993109, @jrtc27 wrote:

In D43293#991986, @himanshu_thechauhan.dev wrote:

@jrtc27 Thanks for your review! I have taken care of your comments. Patch will follow shortly.

Unless I'm mistaken you still use the hartid to index the APLIC, not the APLIC's hart indexes. You need to actually record, and later use, the index <-> cpu mapping when iterating over interrupts-extended.

Whatever hartid is read from the FDT, its logical CPU is kept in target_cpu cpuset. We already, in a way, keep that mapping. When the interrupts are bound, the logical CPU from target_cpu points to the required hart. There is no need to keep separate mapping.

In D43293#993109, @jrtc27 wrote:

In D43293#991986, @himanshu_thechauhan.dev wrote:

@jrtc27 Thanks for your review! I have taken care of your comments. Patch will follow shortly.

Unless I'm mistaken you still use the hartid to index the APLIC, not the APLIC's hart indexes. You need to actually record, and later use, the index <-> cpu mapping when iterating over interrupts-extended.

In D43293#991986, @himanshu_thechauhan.dev wrote:

@jrtc27 Thanks for your review! I have taken care of your comments. Patch will follow shortly.

Changes in v6:

• Added verification of harts that can take interrupt using "interrupts-extended" property of FDT.
• Added target_cpu cpuset which contains the bit map of logical cpus that can take interrupts.
• During binding of the irqs, the cpus from target_cpu cpuset is used.
• Handled failure case and deregister ircs.
• Took care of comments regarding the brackets.
• Added a macro APLIC_IDC to give offset of a given hart.
• Removed return of FILTER_STRAY when IRQ is 0, added a KASSERT instead.
• Use of DEFINE_CLASS_0

@jrtc27 Thanks for your review! I have taken care of your comments. Patch will follow shortly.

In D43293#991694, @jrtc27 wrote:

Also this doesn't properly deal with the interrupts-extended property. I believe that, like the PLIC, you need to iterate over its cells to figure out the mapping between architectural hart ID and APLIC hart index, since they are two independent schemes.