Skip to content

Conversation

@mrjbom
Copy link
Contributor

@mrjbom mrjbom commented Dec 13, 2024

No description provided.

@mrjbom mrjbom changed the title Addition to Update 07_APIC.md Addition to 07_APIC.md Dec 13, 2024
* Bits 32:63: reserved.

Note that the registers are given as a *physical address*, so to access these we will need to map them somewhere in the virtual address space. This is true for the addresses of any I/O APICs we obtain as well. When the system boots, the base address is usually `0xFEE0000` and often this is the value we read from `rdmsr`.
Note that the registers are given as a *physical address*, so to access these we will need to map them somewhere in the virtual address space. This is true for the addresses of any I/O APICs we obtain as well. When the system boots, the base address is usually `0xFEE0000` and often this is the value we read from `rdmsr`. For correct APIC operation, page with Local APIC registers must be mapped as uncachable.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the wording feels a bit clunky, perhaps it could be reflowed as:

For correct operation the local APIC registers should be mapped as 'strong uncachable'.

It may also be worth adding a further sentence to mention how this is setup by the firmware via the MTRRs (at least on the BSP).

Also feel free to add yourself to the list of contributors in 99_Appendices/I_Acknowledgements.md :)

Copy link
Contributor Author

@mrjbom mrjbom Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may also be worth adding a further sentence to mention how this is setup by the firmware via the MTRRs (at least on the BSP).

The osdev wiki says that changing MTRR can cause problems and PAT should be used. In any case, specifying a method seems unnecessary.

Copy link
Member

@DeanoBurrito DeanoBurrito Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The osdev wiki says that changing MTRR can cause problems and PAT should be used. In any case, specifying a method seems unnecessary.

Yeah you should use the PAT, what I was saying is that the firmware should set up the MTRRs (which will override the PAT, ensuring you get the correct caching attributes anyway) - although it generally only does this on the BSP. So yes you're not supposed to touch them in theory, but in practice you need to clone the values from the BSP to the APs. Both bios and uefi make no guarantees about the state of the APs after handing over control of the machine. The osdev wiki is convenient but its no authoritative source.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in any case i'm happy with the PR :)

Copy link
Contributor Author

@mrjbom mrjbom Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DeanoBurrito, Assuming that PAT is supported, which I believe is true for all modern processors, we can neglect MTRR, in which case to set the memory as UC using PAT it is sufficient to set the PCD and PWT bits to 1 in some paging table entry.
The combination of PAT = 0, PCD = 1, PWT = 1 will select the PAT entry that is responsible for UC.
I think this is a fairly simple method, without having to go into MTRR and change the PAT.

@mrjbom mrjbom requested a review from DeanoBurrito December 17, 2024 05:34
@dreamos82
Copy link
Member

@mrjbom i'm happy to merge it, do you want to add yourself in the acknowledgment page here: https://github.com/dreamportdev/Osdev-Notes/blob/master/99_Appendices/I_Acknowledgments.md ?

@dreamos82 dreamos82 merged commit 565fb69 into dreamportdev:master Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants