Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

32bit version #19

Open
jzck opened this issue Apr 15, 2018 · 14 comments
Open

32bit version #19

jzck opened this issue Apr 15, 2018 · 14 comments

Comments

@jzck
Copy link

jzck commented Apr 15, 2018

Hi guys, first of all thanks for the great work,

I'm maintaining a 32bit (aka i386, aka IA-32, aka protected mode) fork of this library my own purposes. It just hit me that everything I've rewritten could be included inside of this crate with conditional compilation ala #[cfg(target_arch="x86-64"]. The main differences obviously concern address sizes, but also page table topology isn't the same in 32bit, and some quirks here and there, nothing huge really.

Your crate is named x86-64 so it's not obvious that these changes would be welcome... should i start writing a draft and make a pull request or should i maintain my own crate on the side?

ps: my version was forked at v0.2.0-alpha-009 so there's not much to catch up with

@lachlansneff
Copy link
Contributor

lachlansneff commented Apr 15, 2018

We were considered breaking up this crate into an osdev crate and conditionally re-exporting osdev_x86_64, and so on. It didn't happen, but if you'd be willing to get that started, I'm all for it.

@phil-opp
Copy link
Member

I'm against conditional compilation in the x86_64 crate because it should be possible to create x86_64 structures on other archtectures too. For example, consider a bootloader that runs in 32-bit mode and wants to do the jump to long mode. With conditional compilation this crate would be useless in that situation because it would only be able to create 32-bit page tables.

As a side note, this crate started as a fork of rz/rust-x86, a library that supports both x86 and x86_64 (without conditional compilation). One reason for the rewrite (albeit not the only one), was that the API ergonomics suffered a lot from this split.

That said, I'm open to a separate crate like @lachlansneff proposes. I wouldn't call it osdev when it only contains CPU abstractions. How about just cpu? (It was still free on crates.io, so I published a dummy crate to reserve that name.)

@phil-opp
Copy link
Member

To avoid code duplication, we could create a x86_common crate that contains things that are identical between 32 and 64 bit. Then x86_64 and the 32-bit crate would just re-export those things. And the cpu crate would use conditonal compilation to either re-export x86_64 or the 32-bit crate, depending on the host platform. It will be still possible to directly depend on x86_64 or the 32-bit crate in case that one doesn't want conditional compilation (e.g. in the bootloader example above).

@jzck
Copy link
Author

jzck commented Apr 15, 2018

so essientially you would like to move everything not specific to 64bit in a x86_common that we can include and share.
AFAIK the similarities between the two modes are essentially on a type level (u32 vs u64)

as an example, where would PhysAddr be defined?

  • in x86_common with a generic for address length
  • in our own crates, where we define address length explicitly

just trying to get an idea as to what exactly would have its place in x86_common. also after giving a go to the rebase I completely agree that conditionnal compilation within the crate is the wong way round of thinking about it, one crate for one cpu mode sounds good.

@phil-opp
Copy link
Member

I just had the idea that we could use cargo features for that. So the x86_common crate could have two features, address_size_32 and address_size_64. Depending on the chosen feature, VirtAddr wraps either an u32 or an u64.

Also, we probably need separate features for virtual and physical addresses, because you can have different sizes with the physical address extension feature.

@jzck
Copy link
Author

jzck commented Apr 15, 2018

although the discrepancy in address sizes between virt/phys with PAE enabled is only true for protected mode, so it should dealt with only in the bottom level protected mode crate and not in x86_common ?

Or maybe x86_common could use cargo features for cpu modes like cpu_long_mode and cpu_protected_mode and it deals itself with the address size logic.

@jzck
Copy link
Author

jzck commented Apr 15, 2018

in any case i'm going to continue maintain my protected mode version and I'll be following the modifications done to your crate and adapting them to my cpu mode.

  • do you guys want to move it into the rust-osdev project?
  • should i push it to crates.io under IA32 or somthing? (I've never used crates.io as an author)

@phil-opp
Copy link
Member

Well, if the x86_common defines both VirtAddr and PhysAddr we need a way to define one as 32 bit and the other as 64 bit.

@phil-opp
Copy link
Member

Moving it to rust-osdev would make sense if the crates are so tightly coupled. You would of course continue to have all permissions on that crate. Or what do you think @lachlansneff?

@lachlansneff
Copy link
Contributor

Seems like a good idea to me

@xacrimon
Copy link

xacrimon commented May 1, 2018

Seems like a good idea to me too!

@Ericson2314
Copy link
Contributor

I was the one who first made https://github.com/gz/rust-x86 multi-mode, breaking things and complicating the APIs. I think a lot of stuff has been rewritten since; maybe the crates can be remerged?

@phil-opp
Copy link
Member

@Ericson2314 While this library originally started out as a fork of rust-x86, most functionality was rewritten or restructured. Looking at the API docs of x86 and x86_64, I don't think that the APIs are similar enough for merging the crates.

@Ericson2314
Copy link
Contributor

@phil-opp Fair enough, I was wondering since they've both evolved a bit since the fork, there might be some incidental converge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants