PCI passthrough API

As discussed here: xapi-project/xapi-project.github.io#286 New methods: - `PCI.disable_dom0_access`: Hide a PCI from dom0 kernel - `PCI.enable_dom0_access`: Unhide a PCI from dom0 kernel - `PCI.get_dom0_access_status`: Return a PCI device dom0 access status The answer is no longer based on a DB field but by comparing the `/proc/cmdline` and the xen cmdline. Deprecated methods: - `PGPU.disable_dom0_acces` - `PGPU.enable_dom0_access` The methods should be called on the PCI belonging to the PGPU instead Deprecated field: - `PGPU.dom0_access` The deprecated methods still work since they now call the PCI methods. The PGPU field is kept up to date with the statuss returned by the PCI methods. Corresponding xe CLI calls have been implemented. Signed-off-by: Benjamin Reis <[email protected]>
xcp-ng · Apr 2, 2024 · bd8ca8a · bd8ca8a
1 parent e808204
commit bd8ca8a
Show file tree

Hide file tree

Showing 12 changed files with 127 additions and 93 deletions.
diff --git a/ocaml/idl/datamodel.ml b/ocaml/idl/datamodel.ml
@@ -6517,6 +6517,20 @@ module Network_sriov = struct
 end
 
 (** PCI devices *)
+let pci_dom0_access =
+  Enum
+    ( "pci_dom0_access"
+    , [
+        ("enabled", "dom0 can access this device as normal")
+      ; ( "disable_on_reboot"
+        , "On host reboot dom0 will be blocked from accessing this device"
+        )
+      ; ("disabled", "dom0 cannot access this device")
+      ; ( "enable_on_reboot"
+        , "On host reboot dom0 will be allowed to access this device"
+        )
+      ]
+    )
 
 module PCI = struct
   let disable_dom0_access =
@@ -6525,6 +6539,7 @@ module PCI = struct
         "Hide a PCI device from the dom0 kernel. (Takes affect after next \
          boot.)"
       ~params:[(Ref _pci, "self", "The PCI to hide")]
+      ~result:(pci_dom0_access, "The accessibility of this PCI from dom0")
       ~allowed_roles:_R_POOL_OP ()
 
   let enable_dom0_access =
@@ -6533,23 +6548,22 @@ module PCI = struct
         "Unhide a PCI device from the dom0 kernel. (Takes affect after next \
          boot.)"
       ~params:[(Ref _pci, "self", "The PCI to unhide")]
+      ~result:(pci_dom0_access, "The accessibility of this PCI from dom0")
       ~allowed_roles:_R_POOL_OP ()
 
-  let is_dom0_access_enabled =
-    call ~name:"is_dom0_access_enabled" ~lifecycle:[]
-      ~doc:
-        "Returns whether the PCI device is accessible from the dom0 kernel on \
-         boot."
+  let get_dom0_access_status =
+    call ~name:"get_dom0_access_status" ~lifecycle:[]
+      ~doc:"Return a PCI device dom0 access status."
       ~params:[(Ref _pci, "self", "The PCI")]
-      ~result:(Bool, "Whether the PCI is accessible from the dom0 kernel")
+      ~result:(pci_dom0_access, "The accessibility of this PCI from dom0")
       ~allowed_roles:_R_POOL_OP ()
 
   let t =
     create_obj ~name:_pci ~descr:"A PCI device" ~doccomments:[]
       ~gen_constructor_destructor:false ~gen_events:true ~in_db:true
       ~lifecycle:[(Published, rel_boston, "")]
       ~messages:
-        [disable_dom0_access; enable_dom0_access; is_dom0_access_enabled]
+        [disable_dom0_access; enable_dom0_access; get_dom0_access_status]
       ~messages_default_allowed_roles:_R_POOL_OP ~persist:PersistEverything
       ~in_oss_since:None ~db_logging:Log_destroy
       ~contents:
@@ -6645,21 +6659,6 @@ end
 (** Physical GPUs (pGPU) *)
 
 module PGPU = struct
-  let dom0_access =
-    Enum
-      ( "pgpu_dom0_access"
-      , [
-          ("enabled", "dom0 can access this device as normal")
-        ; ( "disable_on_reboot"
-          , "On host reboot dom0 will be blocked from accessing this device"
-          )
-        ; ("disabled", "dom0 cannot access this device")
-        ; ( "enable_on_reboot"
-          , "On host reboot dom0 will be allowed to access this device"
-          )
-        ]
-      )
-
   let add_enabled_VGPU_types =
     call ~name:"add_enabled_VGPU_types"
       ~lifecycle:[(Published, rel_vgpu_tech_preview, "")]
@@ -6780,7 +6779,10 @@ module PGPU = struct
 
   let enable_dom0_access =
     call ~name:"enable_dom0_access"
-      ~lifecycle:[(Published, rel_cream, "")]
+      ~lifecycle:[
+          (Published, rel_cream, "")
+        ; (Deprecated, rel_next, "Use PCI.enable_dom0_access instead.")
+      ]
       ~versioned_params:
         [
           {
@@ -6791,12 +6793,15 @@ module PGPU = struct
           ; param_default= None
           }
         ]
-      ~result:(dom0_access, "The accessibility of this PGPU from dom0")
+      ~result:(pci_dom0_access, "The accessibility of this PGPU from dom0")
       ~allowed_roles:_R_POOL_OP ()
 
   let disable_dom0_access =
     call ~name:"disable_dom0_access"
-      ~lifecycle:[(Published, rel_cream, "")]
+      ~lifecycle:[
+          (Published, rel_cream, "")
+        ; (Deprecated, rel_next, "Use PCI.disable_dom0_access instead.")
+      ]
       ~versioned_params:
         [
           {
@@ -6807,7 +6812,7 @@ module PGPU = struct
           ; param_default= None
           }
         ]
-      ~result:(dom0_access, "The accessibility of this PGPU from dom0")
+      ~result:(pci_dom0_access, "The accessibility of this PGPU from dom0")
       ~allowed_roles:_R_POOL_OP ()
 
   let t =
@@ -6868,8 +6873,12 @@ module PGPU = struct
             "A map relating each VGPU type supported on this GPU to the \
              maximum number of VGPUs of that type which can run simultaneously \
              on this GPU"
-        ; field ~qualifier:DynamicRO ~ty:dom0_access
-            ~lifecycle:[(Published, rel_cream, "")]
+        ; field ~qualifier:DynamicRO ~ty:pci_dom0_access
+            ~lifecycle:
+              [
+                  (Published, rel_cream, "")
+                ; (Deprecated, rel_next, "Use PCI.get_dom0_access_status instead.")
+              ]
             ~default_value:(Some (VEnum "enabled")) "dom0_access"
             "The accessibility of this device from dom0"
         ; field ~qualifier:DynamicRO ~ty:Bool

diff --git a/ocaml/idl/schematest.ml b/ocaml/idl/schematest.ml
@@ -3,7 +3,7 @@ let hash x = Digest.string x |> Digest.to_hex
 (* BEWARE: if this changes, check that schema has been bumped accordingly in
    ocaml/idl/datamodel_common.ml, usually schema_minor_vsn *)
 
-let last_known_schema_hash = "186131ad48f40dff30246e8e0c0dbf0a"
+let last_known_schema_hash = "24cc825c8909624fbeac3d06fd3190bb"
 
 let current_schema_hash : string =
   let open Datamodel_types in

diff --git a/ocaml/xapi-cli-server/cli_frontend.ml b/ocaml/xapi-cli-server/cli_frontend.ml
@@ -3698,14 +3698,12 @@ let rec cmdtable_data : (string * cmd_spec) list =
       ; flags= []
       }
     )
-  ; ( "pci-is-dom0-access-enabled"
+  ; ( "pci-get-dom0-access-status"
     , {
         reqd= ["uuid"]
       ; optn= []
-      ; help=
-          "Returns whether the PCI device is accessible from the dom0 kernel \
-           on boot."
-      ; implementation= No_fd Cli_operations.is_dom0_access_enabled
+      ; help= "Return a PCI device dom0 access status."
+      ; implementation= No_fd Cli_operations.get_dom0_access_status
       ; flags= []
       }
     )

diff --git a/ocaml/xapi-cli-server/cli_operations.ml b/ocaml/xapi-cli-server/cli_operations.ml
@@ -1142,7 +1142,7 @@ let gen_cmds rpc session_id =
       )
     ; Client.PGPU.(
         mk get_all_records_where get_by_uuid pgpu_record "pgpu" []
-          ["uuid"; "vendor-name"; "device-name"; "gpu-group-uuid"]
+          ["uuid"; "pci-uuid" ; "vendor-name"; "device-name"; "gpu-group-uuid"]
           rpc session_id
       )
     ; Client.GPU_group.(
@@ -7448,13 +7448,13 @@ let pgpu_enable_dom0_access printer rpc session_id params =
   let uuid = List.assoc "uuid" params in
   let ref = Client.PGPU.get_by_uuid ~rpc ~session_id ~uuid in
   let result = Client.PGPU.enable_dom0_access ~rpc ~session_id ~self:ref in
-  printer (Cli_printer.PMsg (Record_util.pgpu_dom0_access_to_string result))
+  printer (Cli_printer.PMsg (Record_util.pci_dom0_access_to_string result))
 
 let pgpu_disable_dom0_access printer rpc session_id params =
   let uuid = List.assoc "uuid" params in
   let ref = Client.PGPU.get_by_uuid ~rpc ~session_id ~uuid in
   let result = Client.PGPU.disable_dom0_access ~rpc ~session_id ~self:ref in
-  printer (Cli_printer.PMsg (Record_util.pgpu_dom0_access_to_string result))
+  printer (Cli_printer.PMsg (Record_util.pci_dom0_access_to_string result))
 
 let lvhd_enable_thin_provisioning _printer rpc session_id params =
   let sr_uuid = List.assoc "sr-uuid" params in
@@ -7478,25 +7478,23 @@ let lvhd_enable_thin_provisioning _printer rpc session_id params =
        ["sr-uuid"; "initial-allocation"; "allocation-quantum"]
     )
 
-let pci_enable_dom0_access _printer rpc session_id params =
+let pci_enable_dom0_access printer rpc session_id params =
   let uuid = List.assoc "uuid" params in
   let ref = Client.PCI.get_by_uuid ~rpc ~session_id ~uuid in
-  Client.PCI.enable_dom0_access ~rpc ~session_id ~self:ref
+  let result = Client.PCI.enable_dom0_access ~rpc ~session_id ~self:ref in
+  printer (Cli_printer.PMsg (Record_util.pci_dom0_access_to_string result))
 
-let pci_disable_dom0_access _printer rpc session_id params =
+let pci_disable_dom0_access printer rpc session_id params =
   let uuid = List.assoc "uuid" params in
   let ref = Client.PCI.get_by_uuid ~rpc ~session_id ~uuid in
-  Client.PCI.disable_dom0_access ~rpc ~session_id ~self:ref
+  let result = Client.PCI.disable_dom0_access ~rpc ~session_id ~self:ref in
+  printer (Cli_printer.PMsg (Record_util.pci_dom0_access_to_string result))
 
-let is_dom0_access_enabled printer rpc session_id params =
+let get_dom0_access_status printer rpc session_id params =
   let uuid = List.assoc "uuid" params in
   let ref = Client.PCI.get_by_uuid ~rpc ~session_id ~uuid in
-  printer
-    (Cli_printer.PMsg
-       (Bool.to_string
-          (Client.PCI.is_dom0_access_enabled ~rpc ~session_id ~self:ref)
-       )
-    )
+  let result = Client.PCI.get_dom0_access_status ~rpc ~session_id ~self:ref in
+  printer (Cli_printer.PMsg (Record_util.pci_dom0_access_to_string result))
 
 module PVS_site = struct
   let introduce printer rpc session_id params =

diff --git a/ocaml/xapi-cli-server/record_util.ml b/ocaml/xapi-cli-server/record_util.ml
@@ -737,7 +737,7 @@ let host_numa_affinity_policy_of_string = function
            ("Expected 'any', 'best_effort' or 'default_policy', got " ^ s)
         )
 
-let pgpu_dom0_access_to_string x = host_display_to_string x
+let pci_dom0_access_to_string x = host_display_to_string x
 
 let string_to_vdi_onboot s =
   match String.lowercase_ascii s with

diff --git a/ocaml/xapi-cli-server/records.ml b/ocaml/xapi-cli-server/records.ml
@@ -4127,6 +4127,9 @@ let pgpu_record rpc session_id pgpu =
   ; fields=
       [
         make_field ~name:"uuid" ~get:(fun () -> (x ()).API.pGPU_uuid) ()
+      ; make_field ~name:"pci-uuid"
+          ~get:(fun () -> try (xp ()).API.pCI_uuid with _ -> nid)
+          ()
       ; make_field ~name:"vendor-name"
           ~get:(fun () -> try (xp ()).API.pCI_vendor_name with _ -> nid)
           ()
@@ -4135,7 +4138,7 @@ let pgpu_record rpc session_id pgpu =
           ()
       ; make_field ~name:"dom0-access"
           ~get:(fun () ->
-            Record_util.pgpu_dom0_access_to_string (x ()).API.pGPU_dom0_access
+            Record_util.pci_dom0_access_to_string (x ()).API.pGPU_dom0_access
           )
           ()
       ; make_field ~name:"is-system-display-device"

diff --git a/ocaml/xapi/message_forwarding.ml b/ocaml/xapi/message_forwarding.ml
@@ -5884,12 +5884,12 @@ functor
             Client.PCI.enable_dom0_access ~rpc ~session_id ~self
         )
 
-      let is_dom0_access_enabled ~__context ~self =
-        info "PCI.is_dom0_access_enabled: pci = '%s'" (pci_uuid ~__context self) ;
+      let get_dom0_access_status ~__context ~self =
+        info "PCI.get_dom0_access_status: pci = '%s'" (pci_uuid ~__context self) ;
         let host = Db.PCI.get_host ~__context ~self in
-        let local_fn = Local.PCI.is_dom0_access_enabled ~self in
+        let local_fn = Local.PCI.get_dom0_access_status ~self in
         do_op_on ~__context ~local_fn ~host (fun session_id rpc ->
-            Client.PCI.is_dom0_access_enabled ~rpc ~session_id ~self
+            Client.PCI.get_dom0_access_status ~rpc ~session_id ~self
         )
     end
 

diff --git a/ocaml/xapi/xapi_pci.ml b/ocaml/xapi/xapi_pci.ml
@@ -321,12 +321,10 @@ let get_system_display_device () =
   with _ -> None
 
 let disable_dom0_access ~__context ~self =
-  ignore
-  @@ Xapi_pci_helpers.update_dom0_access ~__context ~pci:self ~action:`disable
+  Xapi_pci_helpers.update_dom0_access ~__context ~self ~action:`disable
 
 let enable_dom0_access ~__context ~self =
-  ignore
-  @@ Xapi_pci_helpers.update_dom0_access ~__context ~pci:self ~action:`enable
+  Xapi_pci_helpers.update_dom0_access ~__context ~self ~action:`enable
 
-let is_dom0_access_enabled ~__context ~self =
-  not (Pciops.is_pci_hidden ~__context self)
+let get_dom0_access_status ~__context ~self =
+  Xapi_pci_helpers.determine_dom0_access_status ~__context ~self
diff --git a/ocaml/xapi/xapi_pci.mli b/ocaml/xapi/xapi_pci.mli
@@ -52,11 +52,11 @@ val disable_system_display_device : unit -> unit
 val dequarantine : __context:Context.t -> Xenops_interface.Pci.address -> unit
 (** dequarantine a PCI device. This is idempotent. *)
 
-val disable_dom0_access : __context:Context.t -> self:API.ref_PCI -> unit
+val disable_dom0_access : __context:Context.t -> self:API.ref_PCI -> [`disable_on_reboot | `disabled | `enable_on_reboot | `enabled]
 (** Hide a PCI device from the dom0 kernel. (Takes affect after next boot.) *)
 
-val enable_dom0_access : __context:Context.t -> self:API.ref_PCI -> unit
+val enable_dom0_access : __context:Context.t -> self:API.ref_PCI -> [`disable_on_reboot | `disabled | `enable_on_reboot | `enabled]
 (** Unhide a PCI device from the dom0 kernel. (Takes affect after next boot.) *)
 
-val is_dom0_access_enabled : __context:Context.t -> self:API.ref_PCI -> bool
-(** Check whether a PCI device will be hidden from the dom0 kernel on boot. *)
+val get_dom0_access_status : __context:Context.t -> self:API.ref_PCI -> [`disable_on_reboot | `disabled | `enable_on_reboot | `enabled]
+(** Return a PCI device dom0 access status. *)
diff --git a/ocaml/xapi/xapi_pci_helpers.ml b/ocaml/xapi/xapi_pci_helpers.ml
@@ -15,6 +15,7 @@
 module D = Debug.Make (struct let name = "xapi_pci_helpers" end)
 
 open D
+module Unixext = Xapi_stdext_unix.Unixext
 
 type pci_property = {id: int; name: string}
 
@@ -173,35 +174,62 @@ let igd_is_whitelisted ~__context pci =
   let vendor_id = Db.PCI.get_vendor_id ~__context ~self:pci in
   List.mem vendor_id !Xapi_globs.igd_passthru_vendor_whitelist
 
-let update_dom0_access ~__context ~pci ~action =
-  let db_new =
-    ref
-      ( match action with
-      | `enable ->
-          Pciops.unhide_pci ~__context pci ;
-          `enabled
-      | `disable ->
-          Pciops.hide_pci ~__context pci ;
-          `disabled
-      )
+let is_pci_hidden_cmdline ~__context ~self =
+  let cmdline =
+    match Unixext.read_lines ~path:"/proc/cmdline" with
+    | [x] ->
+        x
+    | _ ->
+        failwith "Unable to read cmdline" ;
   in
-  let expr = Printf.sprintf {|field "PCI"="%s"|} (Ref.string_of pci) in
+  let device = Db.PCI.get_pci_id ~__context ~self in
+  let elems = String.split_on_char ' ' cmdline in
+
+  let pciback = List.find_opt
+    (fun s -> String.starts_with ~prefix:"xen-pciback.hide=" s)
+    elems
+  in
+
+  let contains s1 s2 =
+    let re = Re.Str.regexp_string s2 in
+    try ignore (Re.Str.search_forward re s1 0); true
+    with Not_found -> false
+  in
+
+  match pciback with
+  | None -> false
+  | Some value -> contains value device
+
+let determine_dom0_access_status ~__context ~self =
+  (* Current hidden status *)
+  let is_hidden_cmdline = is_pci_hidden_cmdline ~__context ~self in
+  (* Hidden status after reboot *)
+  let is_hidden = Pciops.is_pci_hidden ~__context self in
+  let new_access =
+    match is_hidden_cmdline, is_hidden with
+    | true, true -> `disabled
+    | false, true -> `disable_on_reboot
+    | false, false -> `enabled
+    | true, false -> `enable_on_reboot
+  in
+
+  (* Keep up to date deprecated PGPU DB field, to eb removed eventually. *)
+  let expr = Printf.sprintf {|field "PCI"="%s"|} (Ref.string_of self) in
   let pgpus = Db.PGPU.get_all_records_where ~__context ~expr in
   List.iter
     (fun (pgpu_ref, _) ->
-      let db_current = Db.PGPU.get_dom0_access ~__context ~self:pgpu_ref in
-      (db_new :=
-         match (db_current, action) with
-         | `enabled, `enable | `disable_on_reboot, `enable ->
-             `enabled
-         | `disabled, `enable | `enable_on_reboot, `enable ->
-             `enable_on_reboot
-         | `enabled, `disable | `disable_on_reboot, `disable ->
-             `disable_on_reboot
-         | `disabled, `disable | `enable_on_reboot, `disable ->
-             `disabled
-      ) ;
-      Db.PGPU.set_dom0_access ~__context ~self:pgpu_ref ~value:!db_new
+      Db.PGPU.set_dom0_access ~__context ~self:pgpu_ref ~value:new_access
     )
     pgpus ;
-  !db_new
+
+  new_access
+
+let update_dom0_access ~__context ~self ~action =
+  ( match action with
+    | `enable ->
+        Pciops.unhide_pci ~__context self
+    | `disable ->
+        Pciops.hide_pci ~__context self
+  ) ;
+
+  determine_dom0_access_status ~__context ~self