| Message ID | 1456919441-101204-6-git-send-email-guangrong.xiao@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 02, 2016 at 07:50:41PM +0800, Xiao Guangrong wrote: > As Igor suggested that we can report the BIOS patched operation region > so that OSPM could see that particular range is in use and be able to > notice conflicts if it happens some day > > Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com> This is reserved RAM, exposing it in _CRS makes no sense to me. > --- > hw/acpi/nvdimm.c | 27 +++++++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c > index 5a17ee2..43fd4c5 100644 > --- a/hw/acpi/nvdimm.c > +++ b/hw/acpi/nvdimm.c > @@ -578,6 +578,7 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets, > GArray *table_data, GArray *linker) > { > Aml *ssdt, *sb_scope, *dev, *field; > + Aml *min_addr, *max_addr, *mr32, *method, *crs; > int mem_addr_offset, nvdimm_ssdt; > > acpi_add_table(table_offsets, table_data); > @@ -602,6 +603,32 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets, > */ > aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0012"))); > > + /* > + * report the dsm memory so that OSPM could see that particular range is > + * in use and be able to notice conflicts if it happens some day. > + */ > + method = aml_method("_CRS", 0, AML_SERIALIZED); > + crs = aml_resource_template(); > + aml_append(crs, aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, > + AML_MAX_FIXED, AML_CACHEABLE, > + AML_READ_WRITE, > + 0, 0x0, 0xFFFFFFFE, 0, > + TARGET_PAGE_SIZE)); > + aml_append(method, aml_name_decl("MR32", crs)); > + mr32 = aml_name("MR32"); > + aml_append(method, aml_create_dword_field(mr32, aml_int(10), "MIN")); > + aml_append(method, aml_create_dword_field(mr32, aml_int(14), "MAX")); > + > + min_addr = aml_name("MIN"); > + max_addr = aml_name("MAX"); > + > + aml_append(method, aml_store(aml_name(NVDIMM_ACPI_MEM_ADDR), min_addr)); > + aml_append(method, aml_add(min_addr, aml_int(TARGET_PAGE_SIZE), > + max_addr)); > + aml_append(method, aml_decrement(max_addr)); > + aml_append(method, aml_return(mr32)); > + aml_append(dev, method); > + > /* map DSM memory and IO into ACPI namespace. */ > aml_append(dev, aml_operation_region("NPIO", AML_SYSTEM_IO, > aml_int(NVDIMM_ACPI_IO_BASE), NVDIMM_ACPI_IO_LEN)); > -- > 1.8.3.1
On 03/03/2016 09:29 PM, Michael S. Tsirkin wrote: > On Wed, Mar 02, 2016 at 07:50:41PM +0800, Xiao Guangrong wrote: >> As Igor suggested that we can report the BIOS patched operation region >> so that OSPM could see that particular range is in use and be able to >> notice conflicts if it happens some day >> >> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com> > > This is reserved RAM, exposing it in _CRS makes no sense to me. As more and more memory will be reserved by BIOS/QEMU, report the information to OSPM and let it check the potential error is bad, no? :)
On Thu, Mar 03, 2016 at 10:05:31PM +0800, Xiao Guangrong wrote: > > > On 03/03/2016 09:29 PM, Michael S. Tsirkin wrote: > >On Wed, Mar 02, 2016 at 07:50:41PM +0800, Xiao Guangrong wrote: > >>As Igor suggested that we can report the BIOS patched operation region > >>so that OSPM could see that particular range is in use and be able to > >>notice conflicts if it happens some day > >> > >>Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com> > > > >This is reserved RAM, exposing it in _CRS makes no sense to me. > > As more and more memory will be reserved by BIOS/QEMU, report the > information to OSPM and let it check the potential error is bad, > no? :) guest has enough info to detect conflicts if it wishes to. IIUC _CRS is not intended for RAM, it's for MMIO resources, if it works for RAM that's an accident.
On Thu, 3 Mar 2016 16:48:55 +0200 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Thu, Mar 03, 2016 at 10:05:31PM +0800, Xiao Guangrong wrote: > > > > > > On 03/03/2016 09:29 PM, Michael S. Tsirkin wrote: > > >On Wed, Mar 02, 2016 at 07:50:41PM +0800, Xiao Guangrong wrote: > > >>As Igor suggested that we can report the BIOS patched operation region > > >>so that OSPM could see that particular range is in use and be able to > > >>notice conflicts if it happens some day > > >> > > >>Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com> > > > > > >This is reserved RAM, exposing it in _CRS makes no sense to me. > > > > As more and more memory will be reserved by BIOS/QEMU, report the > > information to OSPM and let it check the potential error is bad, > > no? :) > > guest has enough info to detect conflicts if it wishes to. > IIUC _CRS is not intended for RAM, it's for MMIO > resources, if it works for RAM that's an accident. If range isn't reserved here, then guest might assume that it's free to use it for a PCI device since PCI0._CRS reports it as available. So we should either reserve range or punch a hole in PCI0._CRS. Reserving ranges is simpler and that's what we've switched to from manual hole punching, see PCI/CPU/Memory hotplug and other motherboard resources.
On Mon, Mar 07, 2016 at 01:16:48PM +0100, Igor Mammedov wrote: > On Thu, 3 Mar 2016 16:48:55 +0200 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Thu, Mar 03, 2016 at 10:05:31PM +0800, Xiao Guangrong wrote: > > > > > > > > > On 03/03/2016 09:29 PM, Michael S. Tsirkin wrote: > > > >On Wed, Mar 02, 2016 at 07:50:41PM +0800, Xiao Guangrong wrote: > > > >>As Igor suggested that we can report the BIOS patched operation region > > > >>so that OSPM could see that particular range is in use and be able to > > > >>notice conflicts if it happens some day > > > >> > > > >>Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com> > > > > > > > >This is reserved RAM, exposing it in _CRS makes no sense to me. > > > > > > As more and more memory will be reserved by BIOS/QEMU, report the > > > information to OSPM and let it check the potential error is bad, > > > no? :) > > > > guest has enough info to detect conflicts if it wishes to. > > IIUC _CRS is not intended for RAM, it's for MMIO > > resources, if it works for RAM that's an accident. > If range isn't reserved here, then guest might assume that it's > free to use it for a PCI device since PCI0._CRS reports it > as available. Does it really? I thought it's guest RAM allocated by BIOS, as opposed to PCI memory. Am I wrong? > So we should either reserve range or punch a hole in PCI0._CRS. > Reserving ranges is simpler and that's what we've switched to > from manual hole punching, see PCI/CPU/Memory hotplug and other > motherboard resources.
On Mon, 7 Mar 2016 14:22:38 +0200 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Mon, Mar 07, 2016 at 01:16:48PM +0100, Igor Mammedov wrote: > > On Thu, 3 Mar 2016 16:48:55 +0200 > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > On Thu, Mar 03, 2016 at 10:05:31PM +0800, Xiao Guangrong wrote: > > > > > > > > > > > > On 03/03/2016 09:29 PM, Michael S. Tsirkin wrote: > > > > >On Wed, Mar 02, 2016 at 07:50:41PM +0800, Xiao Guangrong wrote: > > > > >>As Igor suggested that we can report the BIOS patched operation region > > > > >>so that OSPM could see that particular range is in use and be able to > > > > >>notice conflicts if it happens some day > > > > >> > > > > >>Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com> > > > > > > > > > >This is reserved RAM, exposing it in _CRS makes no sense to me. > > > > > > > > As more and more memory will be reserved by BIOS/QEMU, report the > > > > information to OSPM and let it check the potential error is bad, > > > > no? :) > > > > > > guest has enough info to detect conflicts if it wishes to. > > > IIUC _CRS is not intended for RAM, it's for MMIO > > > resources, if it works for RAM that's an accident. > > If range isn't reserved here, then guest might assume that it's > > free to use it for a PCI device since PCI0._CRS reports it > > as available. > > Does it really? I thought it's guest RAM allocated by BIOS, as opposed > to PCI memory. Am I wrong? Maybe I'm wrong, but aren't RAM and PCI memory mapped into the same physical address space? So what would happen when PCI MMIO BAR would be mapped over above range, since guest thinks it's free to use it as unused resource? > > > So we should either reserve range or punch a hole in PCI0._CRS. > > Reserving ranges is simpler and that's what we've switched to > > from manual hole punching, see PCI/CPU/Memory hotplug and other > > motherboard resources.
On Mon, Mar 07, 2016 at 03:49:47PM +0100, Igor Mammedov wrote: > On Mon, 7 Mar 2016 14:22:38 +0200 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Mon, Mar 07, 2016 at 01:16:48PM +0100, Igor Mammedov wrote: > > > On Thu, 3 Mar 2016 16:48:55 +0200 > > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > > > On Thu, Mar 03, 2016 at 10:05:31PM +0800, Xiao Guangrong wrote: > > > > > > > > > > > > > > > On 03/03/2016 09:29 PM, Michael S. Tsirkin wrote: > > > > > >On Wed, Mar 02, 2016 at 07:50:41PM +0800, Xiao Guangrong wrote: > > > > > >>As Igor suggested that we can report the BIOS patched operation region > > > > > >>so that OSPM could see that particular range is in use and be able to > > > > > >>notice conflicts if it happens some day > > > > > >> > > > > > >>Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com> > > > > > > > > > > > >This is reserved RAM, exposing it in _CRS makes no sense to me. > > > > > > > > > > As more and more memory will be reserved by BIOS/QEMU, report the > > > > > information to OSPM and let it check the potential error is bad, > > > > > no? :) > > > > > > > > guest has enough info to detect conflicts if it wishes to. > > > > IIUC _CRS is not intended for RAM, it's for MMIO > > > > resources, if it works for RAM that's an accident. > > > If range isn't reserved here, then guest might assume that it's > > > free to use it for a PCI device since PCI0._CRS reports it > > > as available. > > > > Does it really? I thought it's guest RAM allocated by BIOS, as opposed > > to PCI memory. Am I wrong? > Maybe I'm wrong, > but aren't RAM and PCI memory mapped into the same physical address space? They are in the same address space but IIRC MMIO has lower priority. > So what would happen when PCI MMIO BAR would be mapped over above range, > since guest thinks it's free to use it as unused resource? IIRC, allocating MMIO BAR over RAM would make the MMIO invisible, irrespective of whether the RAM range is being used for anything. > > > > > > So we should either reserve range or punch a hole in PCI0._CRS. > > > Reserving ranges is simpler and that's what we've switched to > > > from manual hole punching, see PCI/CPU/Memory hotplug and other > > > motherboard resources.
On Mon, 7 Mar 2016 17:09:32 +0200 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Mon, Mar 07, 2016 at 03:49:47PM +0100, Igor Mammedov wrote: > > On Mon, 7 Mar 2016 14:22:38 +0200 > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > On Mon, Mar 07, 2016 at 01:16:48PM +0100, Igor Mammedov wrote: > > > > On Thu, 3 Mar 2016 16:48:55 +0200 > > > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > > > > > On Thu, Mar 03, 2016 at 10:05:31PM +0800, Xiao Guangrong wrote: > > > > > > > > > > > > > > > > > > On 03/03/2016 09:29 PM, Michael S. Tsirkin wrote: > > > > > > >On Wed, Mar 02, 2016 at 07:50:41PM +0800, Xiao Guangrong wrote: > > > > > > >>As Igor suggested that we can report the BIOS patched operation region > > > > > > >>so that OSPM could see that particular range is in use and be able to > > > > > > >>notice conflicts if it happens some day > > > > > > >> > > > > > > >>Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com> > > > > > > > > > > > > > >This is reserved RAM, exposing it in _CRS makes no sense to me. > > > > > > > > > > > > As more and more memory will be reserved by BIOS/QEMU, report the > > > > > > information to OSPM and let it check the potential error is bad, > > > > > > no? :) > > > > > > > > > > guest has enough info to detect conflicts if it wishes to. > > > > > IIUC _CRS is not intended for RAM, it's for MMIO > > > > > resources, if it works for RAM that's an accident. > > > > If range isn't reserved here, then guest might assume that it's > > > > free to use it for a PCI device since PCI0._CRS reports it > > > > as available. > > > > > > Does it really? I thought it's guest RAM allocated by BIOS, as opposed > > > to PCI memory. Am I wrong? > > Maybe I'm wrong, > > but aren't RAM and PCI memory mapped into the same physical address space? > > They are in the same address space but IIRC MMIO has lower priority. > > > So what would happen when PCI MMIO BAR would be mapped over above range, > > since guest thinks it's free to use it as unused resource? > > IIRC, allocating MMIO BAR over RAM would make the MMIO invisible, > irrespective of whether the RAM range is being used for anything. An then driver would start writing 'garbage' to RAM, resulting in strange guest behavior and not work PCI device. that's why reserving region is a good idea if it could be done. > > > > > > > > > > So we should either reserve range or punch a hole in PCI0._CRS. > > > > Reserving ranges is simpler and that's what we've switched to > > > > from manual hole punching, see PCI/CPU/Memory hotplug and other > > > > motherboard resources.
On Mon, Mar 07, 2016 at 05:17:19PM +0100, Igor Mammedov wrote: > > > So what would happen when PCI MMIO BAR would be mapped over above range, > > > since guest thinks it's free to use it as unused resource? > > > > IIRC, allocating MMIO BAR over RAM would make the MMIO invisible, > > irrespective of whether the RAM range is being used for anything. > An then driver would start writing 'garbage' to RAM, resulting in > strange guest behavior and not work PCI device. Do you observe such behaviour? > that's why reserving region is a good idea if it could be done. Which region? Reserve all of RAM in _CRS? > > > > > > > > > > > > > > So we should either reserve range or punch a hole in PCI0._CRS. > > > > > Reserving ranges is simpler and that's what we've switched to > > > > > from manual hole punching, see PCI/CPU/Memory hotplug and other > > > > > motherboard resources.
On Mon, 7 Mar 2016 18:19:46 +0200 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Mon, Mar 07, 2016 at 05:17:19PM +0100, Igor Mammedov wrote: > > > > So what would happen when PCI MMIO BAR would be mapped over above range, > > > > since guest thinks it's free to use it as unused resource? > > > > > > IIRC, allocating MMIO BAR over RAM would make the MMIO invisible, > > > irrespective of whether the RAM range is being used for anything. > > An then driver would start writing 'garbage' to RAM, resulting in > > strange guest behavior and not work PCI device. > > Do you observe such behaviour? > > > that's why reserving region is a good idea if it could be done. > > Which region? Reserve all of RAM in _CRS? Ops, I'm wrong here since PCI0._CRS doesn't include RAM, it's not needed to reserve region there. > > > > > > > > > > > > > > > > > > > So we should either reserve range or punch a hole in PCI0._CRS. > > > > > > Reserving ranges is simpler and that's what we've switched to > > > > > > from manual hole punching, see PCI/CPU/Memory hotplug and other > > > > > > motherboard resources. >
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c index 5a17ee2..43fd4c5 100644 --- a/hw/acpi/nvdimm.c +++ b/hw/acpi/nvdimm.c @@ -578,6 +578,7 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets, GArray *table_data, GArray *linker) { Aml *ssdt, *sb_scope, *dev, *field; + Aml *min_addr, *max_addr, *mr32, *method, *crs; int mem_addr_offset, nvdimm_ssdt; acpi_add_table(table_offsets, table_data); @@ -602,6 +603,32 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets, */ aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0012"))); + /* + * report the dsm memory so that OSPM could see that particular range is + * in use and be able to notice conflicts if it happens some day. + */ + method = aml_method("_CRS", 0, AML_SERIALIZED); + crs = aml_resource_template(); + aml_append(crs, aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, + AML_MAX_FIXED, AML_CACHEABLE, + AML_READ_WRITE, + 0, 0x0, 0xFFFFFFFE, 0, + TARGET_PAGE_SIZE)); + aml_append(method, aml_name_decl("MR32", crs)); + mr32 = aml_name("MR32"); + aml_append(method, aml_create_dword_field(mr32, aml_int(10), "MIN")); + aml_append(method, aml_create_dword_field(mr32, aml_int(14), "MAX")); + + min_addr = aml_name("MIN"); + max_addr = aml_name("MAX"); + + aml_append(method, aml_store(aml_name(NVDIMM_ACPI_MEM_ADDR), min_addr)); + aml_append(method, aml_add(min_addr, aml_int(TARGET_PAGE_SIZE), + max_addr)); + aml_append(method, aml_decrement(max_addr)); + aml_append(method, aml_return(mr32)); + aml_append(dev, method); + /* map DSM memory and IO into ACPI namespace. */ aml_append(dev, aml_operation_region("NPIO", AML_SYSTEM_IO, aml_int(NVDIMM_ACPI_IO_BASE), NVDIMM_ACPI_IO_LEN));
As Igor suggested that we can report the BIOS patched operation region so that OSPM could see that particular range is in use and be able to notice conflicts if it happens some day Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com> --- hw/acpi/nvdimm.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)