CA-418993: Enable PCI ROM BAR before attempting to give permissions to the VM

[freddy77]

Currently the VM is not able to access the ROM as the host ROM BAR is disabled and not set.

[lindig]

I notice that all errors are ignored and none are logged. So how can we understand what is happening?

[freddy77]

No, not the errors but the results of these functions. On error exceptions will be thrown so you will see the error. Here we deal with all virtual files which have specific behaviors. That's why we can (in these specific cases) ignore the result, as we know what is the result already (the kernel return errors in case of failures, not some specific result value).

[lindig]

Why do you prefer using nativeint over a known fixed size, while also using fixed-size constants?

[freddy77]

That's not exactly my choice, the Scanf.sscanf addr "0x%nx 0x%nx 0x%nx" is already using Nativeint types for all these values.

[freddy77]

Would this code be more "clear" ?

            (* Linux disables the ROM BARs if not used and by default does
               not set the start address.
               Force it to set the address using "rom" file.
               This method works also with lockdown mode enabled. *)
            let enable_rom start =
              let finally_close fn flags f =
                let fd = Unix.openfile fn flags 0 in
                finally (fun () -> f fd) (fun () -> Unix.close fd)
              in
              (* read current configured ROM address to check if enabled *)
              let config_fn = Filename.concat sysfs_pci_dev "config" in
              let out = Bytes.make 4 '\x00' in
              let current_addr =
                finally_close config_fn [Unix.O_RDONLY; Unix.O_CLOEXEC]
                  (fun fd ->
                    Unix.(lseek fd 0x30 SEEK_SET) |> ignore ;
                    (* this is a virtual file that control PCI configuration,
                       it will either fail or return 4 *)
                    Unix.(read fd out 0 4) |> ignore ;
                    Nativeint.(
                      logand (of_int32 (Bytes.get_int32_ne out 0)) 0xfffff800n
                    )
                )
              in
              if current_addr <> start then (
                (* to enable output "1" on "rom" file and try to read it *)
                let rom_fn = Filename.concat sysfs_pci_dev "rom" in
                finally_close rom_fn [Unix.O_WRONLY; Unix.O_CLOEXEC] (fun fd ->
                    Unix.(single_write fd (Bytes.of_string "1\n") 0 2) |> ignore
                ) ;
                finally_close rom_fn [Unix.O_RDONLY; Unix.O_CLOEXEC] (fun fd ->
                    (* ignore any error, the ROM could be not correct,
                       just trying to read does the trick *)
                    try Unix.(read fd out 0 4) |> ignore with _ -> ()
                )
              )
            in

[rosslagerwall]

Do you need to do the reverse too? i.e. when unplugging the PCI device from the VM, disable the ROM BAR.

[rosslagerwall]

IIUC this used to work and then it was broken by Secure Boot. Can you explain why (or better put it in the commit message) it was broken and why this fixes it? AFAIK the Linux kernel behaviour of disabling the ROM BARs if not used and not setting the start address has not changed recently.

[freddy77]

Do you need to do the reverse too? i.e. when unplugging the PCI device from the VM, disable the ROM BAR.

Linux always keep the ROM disabled. The problem is having the address more than enabling/disabling. Even if you read the ROM (for instance cat /sys/bus/pci/devices/XXX/rom) Linux enable the ROM in the BAR, read it and then disable again. And even if you could leave the ROM enabled it's not a big deal.

[freddy77]

IIUC this used to work and then it was broken by Secure Boot. Can you explain why (or better put it in the commit message) it was broken and why this fixes it? AFAIK the Linux kernel behaviour of disabling the ROM BARs if not used and not setting the start address has not changed recently.

No, this is broken even for XS8. The VM does not get access to the ROM.

The VM starts but the memory is mapped to a "empty" region, the result is that you always read 0xff bytes. On XS9 we check the address, that's why the VM does not start.

[lindig]

I'd be happy to merge this. If you enjoy OCaml syntax, you can say

Unix.[O_RDONLY; O_CLOEXEC]

[freddy77]

I'd be happy to merge this. If you enjoy OCaml syntax, you can say

Unix.[O_RDONLY; O_CLOEXEC]

It looks nice!
The issue seems the indentation, the make format changed the

                Unixext.with_file config_fn [Unix.O_RDONLY; Unix.O_CLOEXEC] 0

line to

                Unixext.with_file config_fn
                  Unix.[O_RDONLY; O_CLOEXEC]
                  0

Why now it needs all these lines?

[lindig]

I agree; this does not look like a win although it factors out the module nicely. Let's keep it and merge it.

[freddy77]

I agree; this does not look like a win although it factors out the module nicely. Let's keep it and merge it.

Well, I learn something new, it can be useful, thanks.