)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d2fa6bdf7fd6c7a89d68e60278eefd272b233dc6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"1fcdfba0_a78c1d0a","updated":"2026-07-30 11:43:29.000000000","message":"the html report also has a concrancy issue that woudl show up on arm that is proably worht addresing\n\nhttps://minio-api.teim.app/zuul-logs/815/main/8156dad3786e4452bd3e866a16731342/code-review/review-report.html","commit_id":"6e8b7a58a9c090fd90401d10b165d8fc975ef021"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"9950ee51bee8e71dbba4aa2bd0acd2294e16531c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"d15ea126_7e387e19","updated":"2026-07-30 13:32:02.000000000","message":"Need to address few more comments.","commit_id":"36ebf901a01c0fda0f373012ac51c55b0719660e"}],"doc/source/conf.py":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"33ea0e88ca00571a60f98a04e355ea7e006cdd27","unresolved":false,"context_lines":[{"line_number":29,"context_line":"extensions \u003d ["},{"line_number":30,"context_line":"    \u0027sphinx.ext.autodoc\u0027,"},{"line_number":31,"context_line":"    \u0027sphinx.ext.graphviz\u0027,"},{"line_number":32,"context_line":"    \u0027sphinx.ext.todo\u0027,"},{"line_number":33,"context_line":"    \u0027openstackdocstheme\u0027,"},{"line_number":34,"context_line":"    \u0027oslo_config.sphinxconfiggen\u0027,"},{"line_number":35,"context_line":"    \u0027oslo_config.sphinxext\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"0ba34dc8_42d7af71","line":32,"updated":"2026-07-31 08:20:28.000000000","message":"The patch adds \u0027sphinx.ext.todo\u0027 to the extensions list in conf.py and introduces a .. todo:: directive in developer-guide.rst (line 193) discussing whether to maintain pci_sim_vfio_pci solely for UART. However, todo_include_todos\u003dTrue is not set anywhere in conf.py. By default sphinx.ext.todo su...\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: Contributors reading rendered documentation will not see the important design note about evaluating whether pci_sim_vfio_pci should be maintained solely for UART. The note is only visible to those reading the raw RST source, reducing its effectiveness as a design-decision record.\n\n**Suggestion**:\nAdd `todo_include_todos \u003d True` to conf.py (typically near the extensions list) so the TODO block renders in built documentation. Alternatively, convert the .. todo:: block to a regular note or admonition if TODO rendering is intentionally suppressed.","commit_id":"6ca1863b8495096ec1e99e6dd7f8392c6981a6e7"}],"doc/source/contributor/pci-sim/developer-guide.rst":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f65ea13512f873babb2ec421abbdefae624748f9","unresolved":true,"context_lines":[{"line_number":264,"context_line":"variant drivers that need device-specific behavior while reusing the common"},{"line_number":265,"context_line":"VFIO PCI implementation."},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"``pci-sim`` uses a VFIO PCI variant driver named ``pci_sim_vfio_pci``. It"},{"line_number":268,"context_line":"reuses ``vfio-pci-core`` for the normal VFIO PCI machinery and overrides the"},{"line_number":269,"context_line":"parts that must be fake-device aware.  The driver is personality-agnostic;"},{"line_number":270,"context_line":"it dispatches device-specific behavior through the personality ops table"}],"source_content_type":"text/x-rst","patch_set":5,"id":"df1667c6_fbbf1860","line":267,"range":{"start_line":267,"start_character":51,"end_line":267,"end_character":67},"updated":"2026-07-30 11:41:43.000000000","message":"for what its worth\n\neventually we may be able to remvoe this driver and jsut use ``vfio-pci`` similar to how the nvme device emulation works but we will then have a hard depency on the kernel memmap option so for now when we dont need full dma supprot its still good to have for the uart.\n\nwe might just decided later however that maintinign it just to avoid the kernel arges is not useful. we can asses that next cycel","commit_id":"6e8b7a58a9c090fd90401d10b165d8fc975ef021"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"9950ee51bee8e71dbba4aa2bd0acd2294e16531c","unresolved":false,"context_lines":[{"line_number":264,"context_line":"variant drivers that need device-specific behavior while reusing the common"},{"line_number":265,"context_line":"VFIO PCI implementation."},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"``pci-sim`` uses a VFIO PCI variant driver named ``pci_sim_vfio_pci``. It"},{"line_number":268,"context_line":"reuses ``vfio-pci-core`` for the normal VFIO PCI machinery and overrides the"},{"line_number":269,"context_line":"parts that must be fake-device aware.  The driver is personality-agnostic;"},{"line_number":270,"context_line":"it dispatches device-specific behavior through the personality ops table"}],"source_content_type":"text/x-rst","patch_set":5,"id":"518a8511_8e36991f","line":267,"range":{"start_line":267,"start_character":51,"end_line":267,"end_character":67},"in_reply_to":"df1667c6_fbbf1860","updated":"2026-07-30 13:32:02.000000000","message":"Acknowledged, Will add a TODO list with this commit for future work tracking.","commit_id":"6e8b7a58a9c090fd90401d10b165d8fc975ef021"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"e7cc37cbf34ec0a268f14f9fa0b79d3e42cdee58","unresolved":false,"context_lines":[{"line_number":727,"context_line":"  changed after.  The VFIO driver reads ops from the VF device at probe"},{"line_number":728,"context_line":"  time rather than reaching back through the host."},{"line_number":729,"context_line":""},{"line_number":730,"context_line":"``sim-\u003estate`` (``union pci_sim_vf_state``, on ``pci_sim_vfio_vf``)"},{"line_number":731,"context_line":"  Inline per-VF state, typed as a tagged union discriminated by the ops"},{"line_number":732,"context_line":"  pointer.  Replaces the former ``void *personality_data`` heap"},{"line_number":733,"context_line":"  allocation.  Each personality accesses its member directly"}],"source_content_type":"text/x-rst","patch_set":8,"id":"03288808_2ed978aa","line":730,"updated":"2026-07-30 15:23:52.000000000","message":"The developer-guide.rst description for sim-\u003estate says \u0027Replaces the former void *personality_data heap allocation.\u0027 However, the diff shows the old pci_sim_vfio_vf struct had an inline \u0027struct pci_sim_uart uart;\u0027 member — never a void pointer and never heap-allocated. The claim is factually inc...\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Contributors reading the developer guide to understand the design rationale would be misled about the prior architecture, potentially causing confusion when maintaining or extending the personality framework.\n\n**Recommendation**:\nRewrite the description to accurately reflect that the former code used an inline struct pci_sim_uart uart member, and the union was introduced to support multiple personality types without heap allocation. For example: \u0027Each personality accesses its member directly (e.g. \u0026state-\u003euart). The union replaces the former hardcoded inline struct pci_sim_uart member, allowing multiple personality types to share the same per-VF storage without heap allocation.\u0027","commit_id":"bf1fc1fa5e66546a268f2c1d6cb3d0c3b017b578"}],"doc/source/contributor/pci-sim/migration-plan.rst":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"38be51682c76b4e372af627dd58bdbd5051fb450","unresolved":false,"context_lines":[{"line_number":6,"context_line":"migration implementation should preserve enough state that a guest\u0027s"},{"line_number":7,"context_line":"assigned VF continues to function after live migration."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The personality ops-table already provides ``reset`` and"},{"line_number":10,"context_line":"``vfio_open``/``vfio_close`` hooks.  Future save/restore helpers can be"},{"line_number":11,"context_line":"added as ops callbacks (e.g. ``save_state``/``load_state``) so each"},{"line_number":12,"context_line":"personality serialises only its own state."}],"source_content_type":"text/x-rst","patch_set":7,"id":"826e4590_8d01ccab","line":9,"updated":"2026-07-30 15:08:21.000000000","message":"migration-plan.rst states \u0027The personality ops-table already provides reset and vfio_open/vfio_close hooks.\u0027 While the ops struct does declare a reset callback, no code path anywhere in the module calls ops-\u003ereset(). A future contributor could read this document and incorrectly assume the reset m...\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: A contributor implementing live migration may waste time assuming the reset path works, or build on an unwired interface. Low impact since this is future-facing contributor documentation, not user-facing.\n\n**Recommendation**:\nEither wire up ops-\u003ereset() in an appropriate code path (e.g. VFIO FLR or sriov_configure disable), or adjust the migration plan text to say the ops struct \u0027defines a reset callback to be wired up in future phases\u0027 rather than implying it is already functional.","commit_id":"1e5fc06de4e114f68b684ce215b6c4f772cee4ba"}],"pci-sim/fake_pci_sriov.h":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f65ea13512f873babb2ec421abbdefae624748f9","unresolved":true,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"/*"},{"line_number":83,"context_line":" * Personality ops-table: each personality (uart, nvme, ...) fills one"},{"line_number":84,"context_line":" * instance.  The framework dispatches through ops instead of calling"},{"line_number":85,"context_line":" * device-specific functions directly."},{"line_number":86,"context_line":" */"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":5,"id":"e9fd4e1e_67978076","line":84,"range":{"start_line":84,"start_character":28,"end_line":84,"end_character":38},"updated":"2026-07-30 11:41:43.000000000","message":"dispatches is a good way of descirbign this because what pci_sim_personality_ops really is is the c way to define a c++ style abstract base class by manuall building a virutal function table as function pointer in a struct.\n\nso when the personatiy drivers create an instance fo pci_sim_personality_ops they are basiclly creaing an isntance or object of a subclass inthered form an abstrct base class wehre all funciton and data are unimplemtned\n\nthe population of the pci_sim_personality_ops instnace is the constuctor call for the object and the dispatch is just normal dynmic dispatch on the object.\n\nso this is a perfectly normal way to define a interface in c and then provide multiple concreate implmeation of that interface\n\n\nits not a pure interface becasue it also has data member but that why i descibe it as an abstract base class initally.","commit_id":"6e8b7a58a9c090fd90401d10b165d8fc975ef021"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"b68aa996f1351efa268d433fc4168467d153fdbf","unresolved":true,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"/*"},{"line_number":83,"context_line":" * Personality ops-table: each personality (uart, nvme, ...) fills one"},{"line_number":84,"context_line":" * instance.  The framework dispatches through ops instead of calling"},{"line_number":85,"context_line":" * device-specific functions directly."},{"line_number":86,"context_line":" */"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":5,"id":"6a7ec1e4_a5404c2a","line":84,"range":{"start_line":84,"start_character":28,"end_line":84,"end_character":38},"in_reply_to":"e9fd4e1e_67978076","updated":"2026-07-30 14:05:08.000000000","message":"Thanks for the detailed explanation. \n\nThe abstract base class / vtable analogy is a great way to frame it. \n\ndispatches is the correct terminology.","commit_id":"6e8b7a58a9c090fd90401d10b165d8fc975ef021"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f65ea13512f873babb2ec421abbdefae624748f9","unresolved":true,"context_lines":[{"line_number":132,"context_line":"\tstruct fake_pci_device pf;"},{"line_number":133,"context_line":"\tstruct fake_pci_device vfs[MAX_VFS];"},{"line_number":134,"context_line":"\tenum pci_sim_vf_personality personality;"},{"line_number":135,"context_line":"\tconst struct pci_sim_personality_ops *personality_ops;"},{"line_number":136,"context_line":"\tint num_vfs_enabled;"},{"line_number":137,"context_line":"\tint domain_nr;"},{"line_number":138,"context_line":"\tstruct mutex lock; /* Protects VF enable/disable, personality. */"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"b05c71b4_cb09d8db","line":135,"updated":"2026-07-30 11:41:43.000000000","message":"and here becsaue c does not nativly supprot inheritence we\nreplace it with compostion and you can think of this as a c style implmation of the stragtey pattern.\n\npci_sim_personality_ops is defienign the public interface and internal state of the startgy instence which are our device emulation implementions.\n\nhowever this feels slightly off to me\n\nthe pci_sim_personality_ops shoudl be part of the fake_pci_device  stored in the VFs array\n\nit shoudl not be on the fake_pci_host contoler.","commit_id":"6e8b7a58a9c090fd90401d10b165d8fc975ef021"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"9950ee51bee8e71dbba4aa2bd0acd2294e16531c","unresolved":true,"context_lines":[{"line_number":132,"context_line":"\tstruct fake_pci_device pf;"},{"line_number":133,"context_line":"\tstruct fake_pci_device vfs[MAX_VFS];"},{"line_number":134,"context_line":"\tenum pci_sim_vf_personality personality;"},{"line_number":135,"context_line":"\tconst struct pci_sim_personality_ops *personality_ops;"},{"line_number":136,"context_line":"\tint num_vfs_enabled;"},{"line_number":137,"context_line":"\tint domain_nr;"},{"line_number":138,"context_line":"\tstruct mutex lock; /* Protects VF enable/disable, personality. */"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"fb4e0c72_087d4b16","line":135,"in_reply_to":"b05c71b4_cb09d8db","updated":"2026-07-30 13:32:02.000000000","message":"Let me think about both of the comment.","commit_id":"6e8b7a58a9c090fd90401d10b165d8fc975ef021"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"b68aa996f1351efa268d433fc4168467d153fdbf","unresolved":true,"context_lines":[{"line_number":132,"context_line":"\tstruct fake_pci_device pf;"},{"line_number":133,"context_line":"\tstruct fake_pci_device vfs[MAX_VFS];"},{"line_number":134,"context_line":"\tenum pci_sim_vf_personality personality;"},{"line_number":135,"context_line":"\tconst struct pci_sim_personality_ops *personality_ops;"},{"line_number":136,"context_line":"\tint num_vfs_enabled;"},{"line_number":137,"context_line":"\tint domain_nr;"},{"line_number":138,"context_line":"\tstruct mutex lock; /* Protects VF enable/disable, personality. */"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"79326cde_c6e24b03","line":135,"in_reply_to":"fb4e0c72_087d4b16","updated":"2026-07-30 14:05:08.000000000","message":"You\u0027re right. \n\nThe host is just a middleman here. \n\nThe ops describe VF behavior and the VFIO probe is reaching back through\n\nthe host to re-fetch what was already known at VF creation.\n\nLet me fix this part by moving pci_sim_personality_ops into fake_pci_device.","commit_id":"6e8b7a58a9c090fd90401d10b165d8fc975ef021"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f65ea13512f873babb2ec421abbdefae624748f9","unresolved":true,"context_lines":[{"line_number":165,"context_line":"\tstruct vfio_pci_core_device core;"},{"line_number":166,"context_line":"\tstruct mutex lock; /* Serializes VFIO BAR0 access. */"},{"line_number":167,"context_line":"\tconst struct pci_sim_personality_ops *ops;"},{"line_number":168,"context_line":"\tvoid *personality_data;"},{"line_number":169,"context_line":"};"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"/* Cross-file globals (defined in the owning .c, declared extern here) */"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"d2aced1e_4084279f","line":168,"range":{"start_line":168,"start_character":7,"end_line":168,"end_character":23},"updated":"2026-07-30 11:41:43.000000000","message":"this also feeld out of place.\n\nwe are storing the vfio_state_sizein the pci_sim_personality_ops\n\ni feel like the assoated data pointer shoudl also be stored there becied it on lin 93 above.\n\nit also shoudl ideally not be a void* it woudl be better to to model this as\n\na stuct with 2 fields the first field being the lenght adn the seond a pointer to a data buffer that it owns\n\nor better yet a pointer to a tagged union of structs fore each concreat device type.","commit_id":"6e8b7a58a9c090fd90401d10b165d8fc975ef021"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"b68aa996f1351efa268d433fc4168467d153fdbf","unresolved":true,"context_lines":[{"line_number":165,"context_line":"\tstruct vfio_pci_core_device core;"},{"line_number":166,"context_line":"\tstruct mutex lock; /* Serializes VFIO BAR0 access. */"},{"line_number":167,"context_line":"\tconst struct pci_sim_personality_ops *ops;"},{"line_number":168,"context_line":"\tvoid *personality_data;"},{"line_number":169,"context_line":"};"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"/* Cross-file globals (defined in the owning .c, declared extern here) */"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"2ebc9c01_48a7470e","line":168,"range":{"start_line":168,"start_character":7,"end_line":168,"end_character":23},"in_reply_to":"d2aced1e_4084279f","updated":"2026-07-30 14:05:08.000000000","message":"totally agree here. \n\nthis is a closed set in the same .ko and void* is overkill here.\n\nLet me with pointaer to a tagged union of structs.","commit_id":"6e8b7a58a9c090fd90401d10b165d8fc975ef021"}],"pci-sim/fake_pci_sriov_core.c":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8279eddb49d7e99cb61e52338f9adc94d261556f","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"static char *default_personality \u003d \"uart\";"},{"line_number":16,"context_line":"module_param(default_personality, charp, 0444);"},{"line_number":17,"context_line":"MODULE_PARM_DESC(default_personality,"},{"line_number":18,"context_line":"\t\t \"Default VF personality for new PFs (uart, nvme)\");"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"static int fake_intx_irq;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"df655658_fbe43c8e","line":17,"updated":"2026-07-29 14:13:13.000000000","message":"The module parameter description says \u0027Default VF personality for new PFs (uart, nvme)\u0027 but the pci_sim_personalities[] registry only contains the uart personality. A user who loads the module with default_personality\u003dnvme will get an obscure -ENODEV from fake_pci_host_probe.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Users may be confused into trying nvme and getting a module load failure. The error message does mention the unknown personality name, mitigating confusion somewhat.\n\n**Recommendation**:\nRemove \u0027nvme\u0027 from the description until an nvme personality is actually registered, or add a comment noting that nvme is planned but not yet available.","commit_id":"17606fcaa020af418d190cca9ff3573a094b4ff3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4496aa5c6b81531b8ba6d07f7d974c6d0c972cf1","unresolved":true,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"bool vfio_uart_trace;"},{"line_number":28,"context_line":"module_param(vfio_uart_trace, bool, 0644);"},{"line_number":29,"context_line":"MODULE_PARM_DESC(vfio_uart_trace, \"Trace VFIO BAR0 UART register accesses\");"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"static int fake_intx_irq;"},{"line_number":32,"context_line":"module_param(fake_intx_irq, int, 0644);"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"3acd15ea_8f46a24b","side":"PARENT","line":29,"updated":"2026-07-29 17:36:04.000000000","message":"i guess we coudl move the varible defintions into fake_pci_sriov.h if we need too but i do not really want to aspread the module parmat acorss multiple files\n\nso we could also have all the module macros in fake_pci_sriov.h but that is why these are not in fake_pci_sriov_uart.c today","commit_id":"548180334db02379e6ab83cbd8493caf872c55fd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ac38d8c293d0074a6d77788b31a9d8b9f26a6aa4","unresolved":true,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"bool vfio_uart_trace;"},{"line_number":28,"context_line":"module_param(vfio_uart_trace, bool, 0644);"},{"line_number":29,"context_line":"MODULE_PARM_DESC(vfio_uart_trace, \"Trace VFIO BAR0 UART register accesses\");"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"static int fake_intx_irq;"},{"line_number":32,"context_line":"module_param(fake_intx_irq, int, 0644);"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"4b4ba870_0018b418","side":"PARENT","line":29,"in_reply_to":"3acd15ea_8f46a24b","updated":"2026-07-29 17:40:34.000000000","message":"i tired to capture this requirement here \n\nhttps://github.com/openstack/cyborg/blob/master/doc/source/contributor/pci-sim/developer-guide.rst?plain\u003d1#L591-L593\n\nand in the the comment at the top of the fiel as a dsign constraitnt","commit_id":"548180334db02379e6ab83cbd8493caf872c55fd"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"7a9f82bf31188320229404b9bacf3c475e9eab34","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"bool vfio_uart_trace;"},{"line_number":28,"context_line":"module_param(vfio_uart_trace, bool, 0644);"},{"line_number":29,"context_line":"MODULE_PARM_DESC(vfio_uart_trace, \"Trace VFIO BAR0 UART register accesses\");"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"static int fake_intx_irq;"},{"line_number":32,"context_line":"module_param(fake_intx_irq, int, 0644);"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"2e03719c_3aef51e2","side":"PARENT","line":29,"in_reply_to":"4b4ba870_0018b418","updated":"2026-07-30 06:24:15.000000000","message":"Thanks Sean, you\u0027re right, \n\nmodule params should stay centralised in fake_pci_sriov_core.c.\nDone!","commit_id":"548180334db02379e6ab83cbd8493caf872c55fd"}],"pci-sim/fake_pci_sriov_uart.c":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8279eddb49d7e99cb61e52338f9adc94d261556f","unresolved":false,"context_lines":[{"line_number":11,"context_line":""},{"line_number":12,"context_line":"#include \"fake_pci_sriov.h\""},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"static bool vfio_guest_8250_compat;"},{"line_number":15,"context_line":"module_param(vfio_guest_8250_compat, bool, 0644);"},{"line_number":16,"context_line":"MODULE_PARM_DESC(vfio_guest_8250_compat,"},{"line_number":17,"context_line":"\t\t \"Overlay SGI IOC3 serial identity so guest 8250_pci driver binds\");"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"c2ef29e6_7af6f1be","line":14,"updated":"2026-07-29 14:13:13.000000000","message":"The module parameter vfio_guest_8250_compat was moved from core.c (where it was initialized to true) to uart.c (where it has no initializer, defaulting to false). This changes default VFIO BAR0 size from PCI_SIM_VFIO_BAR0_SIZE to BAR0_SIZE and disables the guest config-space SGI IOC3 overlay by d...\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Risk**: Out-of-the-box behavior differs from before: guests will no longer see the SGI IOC3 serial identity in config space and the BAR0 size shrinks from 0x40000 to 0x1000. This will break automated tests or users relying on the default compat overlay.\n\n**Priority**: Before merge\n**Why This Matters**: Out-of-the-box behavior differs from before: guests will no longer see the SGI IOC3 serial identity in config space and the BAR0 size shrinks from 0x40000 to 0x1000. This will break automated tests or users relying on the default compat overlay.\n\n**Recommendation**:\nInitialize the relocated parameter to preserve the original default: \u0027static bool vfio_guest_8250_compat \u003d true;\u0027","commit_id":"17606fcaa020af418d190cca9ff3573a094b4ff3"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8279eddb49d7e99cb61e52338f9adc94d261556f","unresolved":false,"context_lines":[{"line_number":373,"context_line":"\treturn 0;"},{"line_number":374,"context_line":"}"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"static void pci_sim_uart_config_overlay(void *state, char __user *buf,"},{"line_number":377,"context_line":"\t\t\t\t\tloff_t pos, size_t count)"},{"line_number":378,"context_line":"{"},{"line_number":379,"context_line":"\t__le16 val16;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"cee01c06_07ba4698","line":376,"updated":"2026-07-29 14:13:13.000000000","message":"The refactored pci_sim_uart_config_overlay returns void and ignores the return value of pci_sim_uart_copy_config_value, which can return -EFAULT on copy_to_user failure. The old code in vfio.c checked each call and propagated -EFAULT to the caller.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: If copy_to_user fails during config-space overlay (e.g., due to a faulted guest page), the read returns success with a partially corrupted config space instead of -EFAULT. This is a regression in error-handling fidelity for a kernel module.\n\n**Suggestion**:\nChange the config_overlay ops signature to return int (or ssize_t) so that -EFAULT can propagate to the framework dispatcher in pci_sim_vfio_read_config, which already has a path to return errors.","commit_id":"17606fcaa020af418d190cca9ff3573a094b4ff3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6d7491cf5d9875d53eae73410fe14a3f00b36a5d","unresolved":true,"context_lines":[{"line_number":19,"context_line":"static bool vfio_uart_trace;"},{"line_number":20,"context_line":"module_param(vfio_uart_trace, bool, 0644);"},{"line_number":21,"context_line":"MODULE_PARM_DESC(vfio_uart_trace,"},{"line_number":22,"context_line":"\t\t \"Log every VFIO UART register read/write to dmesg\");"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"static DEFINE_XARRAY_ALLOC(pci_sim_tty_xa);"},{"line_number":25,"context_line":"static DEFINE_MUTEX(pci_sim_tty_xa_lock); /* Serializes TTY ID lookup/removal. */"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"6cecb900_2a6b592b","line":22,"updated":"2026-07-29 17:33:23.000000000","message":"all the moudle parmater are intentially centralised core","commit_id":"b3caf01c0166f72396a11a779f94896763fd0202"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"7a9f82bf31188320229404b9bacf3c475e9eab34","unresolved":false,"context_lines":[{"line_number":19,"context_line":"static bool vfio_uart_trace;"},{"line_number":20,"context_line":"module_param(vfio_uart_trace, bool, 0644);"},{"line_number":21,"context_line":"MODULE_PARM_DESC(vfio_uart_trace,"},{"line_number":22,"context_line":"\t\t \"Log every VFIO UART register read/write to dmesg\");"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"static DEFINE_XARRAY_ALLOC(pci_sim_tty_xa);"},{"line_number":25,"context_line":"static DEFINE_MUTEX(pci_sim_tty_xa_lock); /* Serializes TTY ID lookup/removal. */"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"ecc439f4_3413456b","line":22,"in_reply_to":"6cecb900_2a6b592b","updated":"2026-07-30 06:24:15.000000000","message":"Done","commit_id":"b3caf01c0166f72396a11a779f94896763fd0202"}],"pci-sim/fake_pci_sriov_vfio.c":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"6141f7453e4aef82b21abc601f18fdc525ee86a1","unresolved":false,"context_lines":[{"line_number":21,"context_line":"\tif (ret)"},{"line_number":22,"context_line":"\t\treturn ret;"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"\tif (sim-\u003eops-\u003evfio_state_size) {"},{"line_number":25,"context_line":"\t\tsim-\u003epersonality_data \u003d"},{"line_number":26,"context_line":"\t\t\tkzalloc(sim-\u003eops-\u003evfio_state_size, GFP_KERNEL);"},{"line_number":27,"context_line":"\t\tif (!sim-\u003epersonality_data)"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"0ca7bf09_5bb5cd29","line":24,"updated":"2026-07-30 06:25:44.000000000","message":"pci_sim_vfio_open_device calls vfio_pci_core_enable at line 20, but two new error paths introduced by this patch (kzalloc OOM at line 28 and vfio_open failure at line 36) return without calling vfio_pci_core_disable. The upstream VFIO PCI core\u0027s own open_device pattern explicitly calls vfio_pci_c...\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: If kzalloc fails (OOM) or a personality\u0027s vfio_open callback fails after vfio_pci_core_enable has succeeded, the PCI device remains enabled (pci_enable_device, requested regions, etc.) without proper cleanup. Since the VFIO core framework does not call .close_device when .open_device fails, the P...\n\n**Suggestion**:\nAdd a goto cleanup label after vfio_pci_core_finish_enable that calls vfio_pci_core_disable(\u0026sim-\u003ecore), and route both error returns through it. For example: if (!sim-\u003epersonality_data) { ret \u003d -ENOMEM; goto err_disable; } ... if (ret) { kfree(sim-\u003epersonality_data); sim-\u003epersonality_data \u003d NULL; goto err_disable; } ... return 0; err_disable: vfio_pci_core_disable(\u0026sim-\u003ecore); return ret;","commit_id":"83e00cb25a6983e63048f810bc6f8322cbfcecd1"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bcdd590d28d7d740b9cf57eb125666ba4959d3b9","unresolved":false,"context_lines":[{"line_number":21,"context_line":"\tif (ret)"},{"line_number":22,"context_line":"\t\treturn ret;"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"\tif (sim-\u003eops-\u003evfio_state_size) {"},{"line_number":25,"context_line":"\t\tsim-\u003epersonality_data \u003d"},{"line_number":26,"context_line":"\t\t\tkzalloc(sim-\u003eops-\u003evfio_state_size, GFP_KERNEL);"},{"line_number":27,"context_line":"\t\tif (!sim-\u003epersonality_data)"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"4acaf9e5_5bbb5f32","line":24,"in_reply_to":"0ca7bf09_5bb5cd29","updated":"2026-07-30 11:17:05.000000000","message":"damb so the side effect of making some of the schma validation determinstic was i remvoed the constraits on the text form the skills viableity\n\nso its now unfortunetly truncating\n\nyou have already fixed this in v5 with the goto on error so ill resolve this but i need to get back to undoing some of those change i made in the last commit","commit_id":"83e00cb25a6983e63048f810bc6f8322cbfcecd1"}]}
